Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CJS3] client single instance #599

Merged
merged 4 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ export class StreamChat<
ReactionType extends UnknownType = UnknownType,
UserType extends UnknownType = UnknownType
> {
private static _instance?: unknown | StreamChat; // type is undefined|StreamChat, unknown is due to TS limitations with statics

_user?: OwnUserResponse<ChannelType, CommandType, UserType> | UserResponse<UserType>;
activeChannels: {
[key: string]: Channel<
Expand Down Expand Up @@ -171,6 +173,8 @@ export class StreamChat<

/**
* Initialize a client
*
* **Only use constructor for advanced usages. It is strongly advised to use `StreamChat.getInstance()` instead of `new StreamChat()` to reduce integration issues due to multiple WebSocket connections**
* @param {string} key - the api key
* @param {string} [secret] - the api secret
* @param {StreamChatOptions} [options] - additional options, here you can pass custom options to axios instance
Expand Down Expand Up @@ -312,6 +316,125 @@ export class StreamChat<
this.recoverStateOnReconnect = this.options.recoverStateOnReconnect;
}

/**
* Get a client instance
*
* This function always returns the same Client instance to avoid issues raised by multiple Client and WS connections
*
* **After the first call, the client configration will not change if the key or options parameters change**
*
* @param {string} key - the api key
* @param {string} [secret] - the api secret
* @param {StreamChatOptions} [options] - additional options, here you can pass custom options to axios instance
* @param {boolean} [options.browser] - enforce the client to be in browser mode
* @param {boolean} [options.warmUp] - default to false, if true, client will open a connection as soon as possible to speed up following requests
* @param {Logger} [options.Logger] - custom logger
* @param {number} [options.timeout] - default to 3000
* @param {httpsAgent} [options.httpsAgent] - custom httpsAgent, in node it's default to https.agent()
* @example <caption>initialize the client in user mode</caption>
* StreamChat.getInstance('api_key')
* @example <caption>initialize the client in user mode with options</caption>
* StreamChat.getInstance('api_key', { timeout:5000 })
* @example <caption>secret is optional and only used in server side mode</caption>
* StreamChat.getInstance('api_key', "secret", { httpsAgent: customAgent })
*/
public static getInstance<
AttachmentType extends UnknownType = UnknownType,
ChannelType extends UnknownType = UnknownType,
CommandType extends string = LiteralStringForUnion,
EventType extends UnknownType = UnknownType,
MessageType extends UnknownType = UnknownType,
ReactionType extends UnknownType = UnknownType,
UserType extends UnknownType = UnknownType
>(
key: string,
options?: StreamChatOptions,
): StreamChat<
AttachmentType,
ChannelType,
CommandType,
EventType,
MessageType,
ReactionType,
UserType
>;
public static getInstance<
AttachmentType extends UnknownType = UnknownType,
ChannelType extends UnknownType = UnknownType,
CommandType extends string = LiteralStringForUnion,
EventType extends UnknownType = UnknownType,
MessageType extends UnknownType = UnknownType,
ReactionType extends UnknownType = UnknownType,
UserType extends UnknownType = UnknownType
>(
key: string,
secret?: string,
options?: StreamChatOptions,
): StreamChat<
AttachmentType,
ChannelType,
CommandType,
EventType,
MessageType,
ReactionType,
UserType
>;
public static getInstance<
AttachmentType extends UnknownType = UnknownType,
ChannelType extends UnknownType = UnknownType,
CommandType extends string = LiteralStringForUnion,
EventType extends UnknownType = UnknownType,
MessageType extends UnknownType = UnknownType,
ReactionType extends UnknownType = UnknownType,
UserType extends UnknownType = UnknownType
>(
key: string,
secretOrOptions?: StreamChatOptions | string,
options?: StreamChatOptions,
): StreamChat<
AttachmentType,
ChannelType,
CommandType,
EventType,
MessageType,
ReactionType,
UserType
> {
if (!StreamChat._instance) {
if (typeof secretOrOptions === 'string') {
StreamChat._instance = new StreamChat<
AttachmentType,
ChannelType,
CommandType,
EventType,
MessageType,
ReactionType,
UserType
>(key, secretOrOptions, options);
} else {
StreamChat._instance = new StreamChat<
AttachmentType,
ChannelType,
CommandType,
EventType,
MessageType,
ReactionType,
UserType
>(key, secretOrOptions);
}
mahboubii marked this conversation as resolved.
Show resolved Hide resolved
}

return StreamChat._instance as StreamChat<
AttachmentType,
ChannelType,
CommandType,
EventType,
MessageType,
ReactionType,
UserType
>;
}

devToken(userID: string) {
return DevToken(userID);
}
Expand Down
29 changes: 29 additions & 0 deletions test/typescript/unit-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,31 @@ const clientWithoutSecret: StreamChat<
logger: (logLevel: string, msg: string, extraData?: Record<string, unknown>) => {},
});

const singletonClient = StreamChat.getInstance<
AttachmentType,
ChannelType,
CommandType,
EventType,
MessageType,
ReactionType,
UserType
>(apiKey);

const singletonClient1: StreamChat<
{},
ChannelType,
string & {},
{},
{},
{},
UserType
> = StreamChat.getInstance<{}, ChannelType, string & {}, {}, {}, {}, UserType>(apiKey);

const singletonClient2: StreamChat<{}, ChannelType> = StreamChat.getInstance<
{},
ChannelType
>(apiKey, '', {});

const devToken: string = client.devToken('joshua');
const token: string = client.createToken('james', 3600);
const authType: string = client.getAuthType();
Expand All @@ -104,6 +129,10 @@ const updateUsers: Promise<{
users: { [key: string]: UserResponse<UserType> };
}> = client.partialUpdateUsers([updateRequest]);

const updateUsersWithSingletonClient: Promise<{
users: { [key: string]: UserResponse<UserType> };
}> = singletonClient.partialUpdateUsers([updateRequest]);

const eventHandler = (event: Event) => {};
voidReturn = client.on(eventHandler);
voidReturn = client.off(eventHandler);
Expand Down
41 changes: 41 additions & 0 deletions test/unit/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,47 @@ import { StreamChat } from '../../src/client';

const expect = chai.expect;

describe('StreamChat getInstance', () => {
beforeEach(() => {
delete StreamChat._instance;
});

it('instance is stored as static property', () => {
expect(StreamChat._instance).to.be.undefined;

const client = StreamChat.getInstance('key');
expect(client).to.equal(StreamChat._instance);
});

it('always return the same instance', () => {
const client1 = StreamChat.getInstance('key1');
const client2 = StreamChat.getInstance('key1');
const client3 = StreamChat.getInstance('key1');
expect(client1).to.equal(client2);
expect(client2).to.equal(client3);
});

it('changin params has no effect', () => {
const client1 = StreamChat.getInstance('key2');
const client2 = StreamChat.getInstance('key3');

expect(client1).to.equal(client2);
expect(client2.key).to.eql('key2');
});
Copy link
Contributor

@vishalnarkhede vishalnarkhede Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add following scenario to unit-test?

const client1 = StreamChat.getInstance('key2');
await client1.setUser({ id: 'vishal });
const client2 = StreamChat.getInstance('key3'); // or key2
await client2.setUser({ id: 'vishal });

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently if you call setUser twice, it throws following error:

'Use client.disconnect() before trying to connect as a different user. connectUser was called twice.'

But I think we should allow it if current user is same as id provided to setUser

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed a test d2fcfc3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vishalnarkhede yea I actually put a comment in the PR description about this which is out of the scope of this PR. This is still an integration mistake but we can make it easier for users by not throwing error

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahaa ... sorry missed it!!

Can we add it though ... seems like a simple check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea totally will open another PR on top of this one


it('should throw error if connectUser called twice on an instance', async () => {
const client1 = StreamChat.getInstance('key2', { allowServerSideConnect: true });
client1._setupConnection = () => Promise.resolve();
client1._setToken = () => Promise.resolve();

await client1.connectUser({ id: 'vishal' }, 'token');
const client2 = StreamChat.getInstance('key2');
expect(() => client2.connectUser({ id: 'Amin' }, 'token')).to.throw(
/connectUser was called twice/,
);
});
});

describe('Client userMuteStatus', function () {
const client = new StreamChat('', '');
const user = { id: 'user' };
Expand Down