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

fix!: replace StreamChatGenerics with module augmentation #1458

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

arnautov-anton
Copy link
Contributor

@arnautov-anton arnautov-anton commented Jan 30, 2025

Description of the changes, What, Why and How?

BREAKING CHANGES:

  • dropped StreamChatGenerics, use Custom<InterfaceKey> to extend your types
  • type InviteOptions has been renamed to UpdateChannelOptions
  • type UpdateChannelOptions has been renamed to UpdateChannelTypeOptions
  • type ThreadResponseCustomData has been renamed to CustomThreadData
  • type MarkAllReadOptions has been deleted in favour of type MarkChannelsReadOptions
  • type QueryFilter no longer supports $ne and $nin operators
  • type ChannelMembership has been deleted in favour of type ChannelMemberResponse
  • function formatMessage (utils.ts) no longer returns __html property in the formatted message output

FIXES:

  • channel_role has been removed from the type of the members parameter of the Channel.inviteMembers method

Copy link
Contributor

github-actions bot commented Jan 30, 2025

Size Change: -632 B (-0.12%)

Total Size: 523 kB

Filename Size Change
dist/browser.es.js 114 kB -209 B (-0.18%)
dist/browser.full-bundle.min.js 62.8 kB +114 B (+0.18%)
dist/browser.js 116 kB -163 B (-0.14%)
dist/index.es.js 115 kB -211 B (-0.18%)
dist/index.js 116 kB -163 B (-0.14%)

compressed-size-action

src/types.ts Outdated

export interface CustomAttachmentType {}
export interface CustomChannelType {}
export interface CustomCommandType {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might have to figure out a better way for this to work. Currently using keyof CustomCommandType to make it work with interface merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This right here: https://github.com/GetStream/stream-chat-js/pull/1458/files#diff-c54113cf61ec99691748a3890bfbeb00e10efb3f0a76f03a0fd9ec49072e410aR2124

Actual module augmentation would have to look something like:

// file.d.ts

import 'stream-chat';

declare module 'stream-chat' {
    interface CustomCommandType {
        customCommand: unknown;
        otherCustomCommand: unknown;
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good ! We're already doing similar things for other parts of the RN SDK (extending functionality and overloads) so this will come natural.

The entire change is breaking I assume, right ? What's the strategy to go about merging this when the time comes ? I suppose all SDKs would need to bump their major versions if they wish to upgrade their version of stream-chat. Although kind of large in size, the change seems easy to do (if generics were being used in the first place).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in case of React SDK we'd be releasing v13.0.0 which will bump its stream-chat peer to v9.0.0 - the peer of the SDK is currently bound to v8-something.

As for the migration - see this draft of a migration guide here, feel free to suggest enhancements. Not sure where it'll live yet, I assume both React and RN will be similar in terms of migration but I'm open to adding tabs to switch between RN and React.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair - in that case we should pick it up earlier rather than later since stream-chat is a direct dependency for stream-chat-react-native (and not a peer one).

Since we'll likely have to do a V7 to go along with it as well, maybe we can do the same ? it should get rid of our pesky TS errors when there's a version mismatch (despite the fact it's a direct dependency).

fyi @oliverlaz

Copy link
Member

Choose a reason for hiding this comment

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

@arnautov-anton arnautov-anton force-pushed the fix/remove-stream-chat-generics branch from 9765037 to ad95891 Compare January 30, 2025 22:05
src/poll.ts Outdated Show resolved Hide resolved
const updateKey = key as keyof typeof _user;

if (updateKey in event.user) {
// @ts-expect-error it has an issue with this, not sure why
Copy link
Contributor Author

Choose a reason for hiding this comment

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

😬

@arnautov-anton arnautov-anton force-pushed the fix/remove-stream-chat-generics branch from ad95891 to fe6b938 Compare January 31, 2025 08:57
@arnautov-anton arnautov-anton changed the title fix!: replace StreamChatGenerics with interface merging fix!: replace StreamChatGenerics with module augmentation Jan 31, 2025
@arnautov-anton arnautov-anton force-pushed the fix/remove-stream-chat-generics branch from fe6b938 to 709bbb5 Compare February 4, 2025 14:22
src/types.ts Outdated Show resolved Hide resolved
@arnautov-anton arnautov-anton force-pushed the fix/remove-stream-chat-generics branch 4 times, most recently from efe8430 to 8a87f88 Compare February 12, 2025 17:10
@arnautov-anton arnautov-anton force-pushed the fix/remove-stream-chat-generics branch from ca04832 to 9ae0456 Compare February 13, 2025 13:53
@arnautov-anton arnautov-anton force-pushed the fix/remove-stream-chat-generics branch from 9ae0456 to f486ff3 Compare February 13, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants