Skip to content

feat: omit 'appId' from SendbirdChatInitParams #693

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 21, 2023
Merged

feat: omit 'appId' from SendbirdChatInitParams #693

merged 1 commit into from
Jul 21, 2023

Conversation

@AhyoungRyu
Copy link
Contributor

@AhyoungRyu AhyoungRyu commented Jul 21, 2023

Addressing a feedback from @sravan-s after #692 merged. appId is better not to override just to be safe.

@AhyoungRyu AhyoungRyu self-assigned this Jul 21, 2023
@AhyoungRyu AhyoungRyu requested a review from sravan-s July 21, 2023 05:06
AhyoungRyu
AhyoungRyu commented Jul 21, 2023
View reviewed changes
src/lib/hooks/useConnect/setupConnection.ts
],
newInstance: true,
...sdkInitParams,
...(sdkInitParams ?? {}),
Copy link
Contributor Author

@AhyoungRyu AhyoungRyu Jul 21, 2023

Choose a reason for hiding this comment

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

@sravan-s So weird that
Also, a2f0541 this commit from #692 is gone. So had to change this again 🤷‍♂️

Copy link
Contributor

@sravan-s sravan-s Jul 21, 2023

Choose a reason for hiding this comment

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

yeah weird

sravan-s
sravan-s reviewed Jul 21, 2023
View reviewed changes
src/lib/types.ts
}>;

export type SendbirdChatInitParams = SendbirdChatParams<Module[]>;
export type SendbirdChatInitParams = Omit<SendbirdChatParams<Module[]>, 'appId'>;
Copy link
Contributor

@sravan-s sravan-s Jul 21, 2023

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@sravan-s sravan-s Jul 21, 2023

Choose a reason for hiding this comment

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

should be same in https://github.com/sendbird/sendbird-uikit-react/blob/main/scripts/index_d_ts#L116

@AhyoungRyu AhyoungRyu merged commit 8d0cd00 into main Jul 21, 2023
@AhyoungRyu AhyoungRyu deleted the feat/AC-217/sdk-init-params-type branch July 21, 2023 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@sravan-s sravan-s sravan-s approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@AhyoungRyu AhyoungRyu

Labels

Ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AhyoungRyu @sravan-s