Skip to content
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
3 changes: 2 additions & 1 deletion ai-docs/v10-followups.md
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,15 @@ These are full items in [Dead code and leftovers](#dead-code); listed here so th

<a id="watchers-members-in-place"></a>

#### Watchers and members are mutated in place on the store-backed state
#### Watchers and members are mutated in place on the store-backed state [SOLVED 14301018]

`bug` · confidence: high · pre-release: no · channel state

- **Where:** `src/channel.ts:2530, 2536` (`user.watching.start/stop`, `user.updated`), `:2846, :2855` (`user.banned/unbanned`), `:2950` (`_initializeState` watchers); getters `src/channel_state.ts:322-328`
- **What:** `channelState.watchers[id] = user`, `delete channelState.watchers[id]` and `channelState.members[id] = {...}` write into the store's current object; no new reference is published.
- **Evidence / scenario:** `useStateStore(channel.state, (s) => ({ watchers: s.watchers }))` does not re-render when another user starts watching or is banned; the shallow selector compare sees the same object.
- **Suggested change:** Assign new objects (`channelState.watchers = { ...channelState.watchers, [id]: user }`, same for `members`) or use `partialNext`.
- **Also:** `member.removed` for the current user left `membership` set; it now resets it to the empty initial value.

<a id="read-events-gate"></a>

Expand Down
58 changes: 35 additions & 23 deletions src/channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2359,13 +2359,18 @@ export class Channel extends ChannelApi {
case 'user.watching.start':
case 'user.updated':
if (event.user?.id) {
channelState.watchers[event.user.id] = event.user;
channelState.watchers = {
...channelState.watchers,
[event.user.id]: event.user,
};
channel.getClient().state.updateUserReference(event.user, channel.cid);
}
break;
case 'user.watching.stop':
if (event.user?.id) {
delete channelState.watchers[event.user.id];
if (event.user?.id && channelState.watchers[event.user.id]) {
const newWatchers = { ...channelState.watchers };
delete newWatchers[event.user.id];
channelState.watchers = newWatchers;
}
break;
case 'message.deleted':
Expand Down Expand Up @@ -2564,19 +2569,26 @@ export class Channel extends ChannelApi {
}
break;
}
case 'member.removed':
if (event.user?.id) {
case 'member.removed': {
// The spec documents `event.user` as the user who removed the member, so the removed
// user is read from `event.member` first.
const removedUserId =
event.member?.user?.id ?? event.member?.user_id ?? event.user?.id;
if (removedUserId) {
const newMembers = {
...channelState.members,
};

delete newMembers[event.user.id];
delete newMembers[removedUserId];

channelState.members = newMembers;

// TODO?: unset membership
if (removedUserId === this.getClient().userId) {
channelState.membership = {} as ChannelMemberResponse;
}
}
break;
}
case 'notification.mark_unread': {
const ownMessage = event.user?.id === this.getClient().user?.id;
if (!ownMessage || !event.user || event.last_read_at == null) break;
Expand Down Expand Up @@ -2660,23 +2672,21 @@ export class Channel extends ChannelApi {
break;
}
case 'user.banned':
case 'user.unbanned': {
if (!event.user?.id) break;
channelState.members[event.user.id] = {
...(channelState.members[event.user.id] || {}),
shadow_banned: !!event.shadow,
banned: !event.shadow,
user: { ...(channelState.members[event.user.id]?.user || {}), ...event.user },
};
break;
case 'user.unbanned':
if (!event.user?.id) break;
channelState.members[event.user.id] = {
...(channelState.members[event.user.id] || {}),
shadow_banned: false,
banned: false,
user: { ...(channelState.members[event.user.id]?.user || {}), ...event.user },
const isBanned = event.type === 'user.banned';
const member = channelState.members[event.user.id];
channelState.members = {
...channelState.members,
[event.user.id]: {
...(member || {}),
shadow_banned: isBanned && !!event.shadow,
banned: isBanned && !event.shadow,
user: { ...(member?.user || {}), ...event.user },
},
};
break;
}
default:
}

Expand Down Expand Up @@ -2754,13 +2764,15 @@ export class Channel extends ChannelApi {
if (state.watcher_count !== undefined) {
this.state.watcher_count = state.watcher_count;
}
if (state.watchers) {
if (state.watchers?.length) {
const watchers = { ...this.state.watchers };
for (const watcher of state.watchers) {
if (watcher) {
clientState.updateUserReference(watcher, this.cid);
this.state.watchers[watcher.id] = watcher;
watchers[watcher.id] = watcher;
}
}
this.state.watchers = watchers;
}

// Initialized to the latest message (or now, if empty) so nothing up to here counts as
Expand Down
164 changes: 164 additions & 0 deletions test/unit/channel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1524,6 +1524,148 @@ describe('Channel _handleChannelEvent', function () {

expect(channel.data.member_count).to.equal(5);
});

it('member.removed for the current user clears ChannelState.membership', () => {
const currentMember = generateMember({ user });
const otherMember = generateMember({ user: otherUser });
channel._handleChannelEvent({ type: 'member.added', user, member: currentMember });
channel._handleChannelEvent({
type: 'member.added',
user: otherUser,
member: otherMember,
});

channel._handleChannelEvent({
type: 'member.removed',
user: otherUser,
member: otherMember,
});
expect(channel.state.membership).to.deep.equal(currentMember);

channel._handleChannelEvent({
type: 'member.removed',
user,
member: currentMember,
});
expect(channel.state.members).to.be.empty;
expect(channel.state.membership).to.be.empty;
});

it('member.removed removes event.member, not an event.user who removed them', () => {
const currentMember = generateMember({ user });
const otherMember = generateMember({ user: otherUser });
channel._handleChannelEvent({ type: 'member.added', user, member: currentMember });
channel._handleChannelEvent({
type: 'member.added',
user: otherUser,
member: otherMember,
});

channel._handleChannelEvent({ type: 'member.removed', user, member: otherMember });

expect(channel.state.members).to.have.keys([user.id]);
expect(channel.state.membership).to.deep.equal(currentMember);
});

it('member.removed falls back to event.user without event.member', () => {
channel._handleChannelEvent({
type: 'member.added',
user: otherUser,
member: generateMember({ user: otherUser }),
});

channel._handleChannelEvent({ type: 'member.removed', user: otherUser });

expect(channel.state.members).to.be.empty;
});
});

describe('watchers and members publish a new reference', () => {
const subscribeTo = (key) => {
const handler = vi.fn();
channel.state.subscribeWithSelector((state) => ({ [key]: state[key] }), handler);
handler.mockClear();
return handler;
};

it('notifies watchers subscribers on user.watching.start and user.watching.stop', () => {
const handler = subscribeTo('watchers');

channel._handleChannelEvent({ type: 'user.watching.start', user: otherUser });
expect(handler).toHaveBeenCalledTimes(1);
expect(channel.state.watchers).to.deep.equal({ [otherUser.id]: otherUser });

channel._handleChannelEvent({ type: 'user.watching.stop', user: otherUser });
expect(handler).toHaveBeenCalledTimes(2);
expect(channel.state.watchers).to.be.empty;
});

it('notifies watchers subscribers on user.updated for a watcher', () => {
channel._handleChannelEvent({ type: 'user.watching.start', user: otherUser });
const handler = subscribeTo('watchers');
const renamed = { ...otherUser, name: 'Renamed' };

channel._handleChannelEvent({ type: 'user.updated', user: renamed });

expect(handler).toHaveBeenCalledTimes(1);
expect(channel.state.watchers[otherUser.id]).to.equal(renamed);
});

it('does not notify watchers subscribers on user.watching.stop for a user who was not watching', () => {
const handler = subscribeTo('watchers');

channel._handleChannelEvent({ type: 'user.watching.stop', user: otherUser });

expect(handler).not.toHaveBeenCalled();
});

it('notifies members subscribers on user.banned and user.unbanned', () => {
channel._handleChannelEvent({
type: 'member.added',
user: otherUser,
member: generateMember({ user: otherUser }),
});
const handler = subscribeTo('members');

channel._handleChannelEvent({ type: 'user.banned', user: otherUser });
expect(handler).toHaveBeenCalledTimes(1);
expect(channel.state.members[otherUser.id].banned).to.be.true;

channel._handleChannelEvent({ type: 'user.unbanned', user: otherUser });
expect(handler).toHaveBeenCalledTimes(2);
expect(channel.state.members[otherUser.id].banned).to.be.false;
});

it('notifies membership subscribers when the current user is removed', () => {
channel._handleChannelEvent({
type: 'member.added',
user,
member: generateMember({ user }),
});
const handler = subscribeTo('membership');

channel._handleChannelEvent({ type: 'member.removed', user });

expect(handler).toHaveBeenCalledTimes(1);
});

it('does not notify membership subscribers when another member is removed', () => {
channel._handleChannelEvent({
type: 'member.added',
user,
member: generateMember({ user }),
});
channel._handleChannelEvent({
type: 'member.added',
user: otherUser,
member: generateMember({ user: otherUser }),
});
const handler = subscribeTo('membership');

channel._handleChannelEvent({ type: 'member.removed', user: otherUser });

expect(handler).not.toHaveBeenCalled();
});
});

describe('message.new', () => {
Expand Down Expand Up @@ -4000,6 +4142,28 @@ describe('Channel last_message_at', () => {
});

describe('Channel _initializeState', () => {
it('publishes a new watchers reference, keeping watchers already in state', async () => {
const client = await getClientWithUser();
const channel = client.channelManager.ensure({ type: 'messaging', id: uuidv4() });
const alice = { id: 'alice' };
const bob = { id: 'bob' };
channel.state.watchers = { [alice.id]: alice };
const previousWatchers = channel.state.watchers;
const handler = vi.fn();
channel.state.subscribeWithSelector(
(state) => ({ watchers: state.watchers }),
handler,
);
handler.mockClear();

channel._initializeState({ watchers: [bob] });

expect(handler).toHaveBeenCalledTimes(1);
expect(channel.state.watchers).to.not.equal(previousWatchers);
expect(channel.state.watchers).to.deep.equal({ [alice.id]: alice, [bob.id]: bob });
expect(previousWatchers).to.deep.equal({ [alice.id]: alice });
});

it('should not keep members that have unwatched since last watch', async () => {
const client = await getClientWithUser();
const channel = client.channelManager.ensure({ type: 'messaging', id: uuidv4() });
Expand Down
Loading