diff --git a/.changelog/6049-fix-activity-graph-catch-up.md b/.changelog/6049-fix-activity-graph-catch-up.md new file mode 100644 index 0000000000..cc1507ed4d --- /dev/null +++ b/.changelog/6049-fix-activity-graph-catch-up.md @@ -0,0 +1,3 @@ +### Fixed + +- Fixed development builds throwing when a React 19.2 `` shows Web Chat again, in PR [#6049](https://github.com/microsoft/BotFramework-WebChat/pull/6049), by [@OEvgeny](https://github.com/OEvgeny) diff --git a/__tests__/html2/react19/activity.hideShow.transcript.html b/__tests__/html2/react19/activity.hideShow.transcript.html new file mode 100644 index 0000000000..1f45e9cc0a --- /dev/null +++ b/__tests__/html2/react19/activity.hideShow.transcript.html @@ -0,0 +1,98 @@ + + + + React 19: Web Chat keeps its transcript when an <Activity> hides and shows it + + + +
+ + + + diff --git a/__tests__/html2/react19/activity.hideShow.transcript.html.snap-1.png b/__tests__/html2/react19/activity.hideShow.transcript.html.snap-1.png new file mode 100644 index 0000000000..041fcd2500 Binary files /dev/null and b/__tests__/html2/react19/activity.hideShow.transcript.html.snap-1.png differ diff --git a/__tests__/html2/react19/activity.hideShow.transcript.sameElement.html b/__tests__/html2/react19/activity.hideShow.transcript.sameElement.html new file mode 100644 index 0000000000..4d8debae0f --- /dev/null +++ b/__tests__/html2/react19/activity.hideShow.transcript.sameElement.html @@ -0,0 +1,92 @@ + + + + React 19: Web Chat shows activities added while an <Activity> hid it, without rendering on show + + + +
+ + + + diff --git a/__tests__/html2/react19/activity.hideShow.transcript.sameElement.html.snap-1.png b/__tests__/html2/react19/activity.hideShow.transcript.sameElement.html.snap-1.png new file mode 100644 index 0000000000..041fcd2500 Binary files /dev/null and b/__tests__/html2/react19/activity.hideShow.transcript.sameElement.html.snap-1.png differ diff --git a/packages/core/src/graph/createGraphFromStore.spec.ts b/packages/core/src/graph/createGraphFromStore.spec.ts index 00f7362666..ced808113a 100644 --- a/packages/core/src/graph/createGraphFromStore.spec.ts +++ b/packages/core/src/graph/createGraphFromStore.spec.ts @@ -15,7 +15,7 @@ const incoming = (id: string) => ({ type RawJSON = readonly { readonly '@value': { readonly id: string } }[] | undefined; -function activityIdsOf(graph: ReturnType): readonly string[] { +function activityIdsOf(graph: Pick, 'getState'>): readonly string[] { return Array.from(graph.getState().values()).flatMap( node => (node['urn:microsoft:webchat:direct-line-activity:raw-json'] as RawJSON)?.map(({ '@value': { id } }) => id) ?? [] @@ -83,17 +83,15 @@ scenario('createGraphFromStore()', bdd => { return outcome; }) - .then('the graph should not follow the store anymore', ({ subscriptions }, { graph, subscriber1 }) => { + .then('the graph should not follow the store anymore', ({ subscriptions }, { subscriber1 }) => { + // Does not read the graph: a read would catch up with the store, before the subscription below does. expect(subscriptions.size).toBe(0); - expect(activityIdsOf(graph)).toEqual(['a-1', 'a-2']); expect(subscriber1).toHaveBeenCalledTimes(1); }) .when('a subscriber subscribes again', (_, { graph }) => { const subscriber = jest.fn(); - graph.subscribe(subscriber); - - return { graph, subscriber }; + return { graph, subscriber, unsubscribe: graph.subscribe(subscriber) }; }) .then( 'the graph should catch up with the store and notify the subscriber', @@ -102,5 +100,53 @@ scenario('createGraphFromStore()', bdd => { expect(subscriber).toHaveBeenCalledTimes(1); expect(subscriptions.size).toBe(1); } + ) + // A hidden (React 19.2+) runs the cleanup of effects, so the graph has no subscribers. When the host shows + // Web Chat again with a new element, it renders and reads the graph before its effects subscribe again. + .when('the subscriber unsubscribes, then an activity is received', ({ store }, outcome) => { + outcome.unsubscribe(); + store.dispatch(incoming('a-4')); + + return outcome; + }) + .then( + 'reading the graph should catch up with the store, without subscribing to it', + ({ subscriptions }, { graph, subscriber }) => { + expect(activityIdsOf(graph)).toEqual(['a-1', 'a-2', 'a-3', 'a-4']); + expect(subscriptions.size).toBe(0); + expect(subscriber).toHaveBeenCalledTimes(1); + } + ) + .and('reading the graph again should return the same state', (_, { graph }) => + expect(graph.getState()).toBe(graph.getState()) ); + + bdd + .given('a graph without subscribers, created from a store with an activity', () => { + const store = createStore(); + + store.dispatch(incoming('a-1')); + + return { graph: createGraphFromStore(store), store }; + }) + .when('an activity is received, then a transaction is made on the graph', ({ graph, store }) => { + let activityIdsInTransaction: readonly string[] = []; + + store.dispatch(incoming('a-2')); + + // The transaction reads the graph (auto-inversion) before the graph caught up with the store. + graph.act(graph => { + activityIdsInTransaction = activityIdsOf(graph); + graph.upsert({ '@id': '_:m1', '@type': 'MessageLike', text: 'Hello, World!' }); + }); + + return { activityIdsInTransaction, graph }; + }) + .then('the transaction should see both activities', (_, { activityIdsInTransaction }) => + expect(activityIdsInTransaction).toEqual(['a-1', 'a-2']) + ) + .and('the graph should have both activities and the node', (_, { graph }) => { + expect(activityIdsOf(graph)).toEqual(['a-1', 'a-2']); + expect(graph.getState().has('_:m1')).toBe(true); + }); }); diff --git a/packages/core/src/graph/createGraphFromStore.ts b/packages/core/src/graph/createGraphFromStore.ts index 88e9d6b2b2..0847a87e82 100644 --- a/packages/core/src/graph/createGraphFromStore.ts +++ b/packages/core/src/graph/createGraphFromStore.ts @@ -7,7 +7,20 @@ import type { Activity } from '../reducers/activities/sort/types'; function createGraphFromStore(store: ReturnType): SlantGraph { const graph = new SlantGraph(); + const actOnGraph = graph.act.bind(graph); let prevActivities: readonly Activity[] | undefined; + let transacting = false; + + // act() is not re-entrant: reads inside a transaction (e.g. by middleware) must not catch up with the store. + const transact: typeof graph.act = fn => { + transacting = true; + + try { + actOnGraph(fn); + } finally { + transacting = false; + } + }; // TODO: [P0] Except channel audience, we should be specific about all audience, e.g. name. const channelAudience = parse(SlantNodeSchema, { @@ -46,7 +59,7 @@ function createGraphFromStore(store: ReturnType): SlantGraph // TODO: [P*] Supports deleting node from the graph. const addedActivities = activitySet.difference(prevActivitySet); - graph.act(graph => { + transact(graph => { for (const activity of addedActivities) { const { from: { role } @@ -121,9 +134,24 @@ function createGraphFromStore(store: ReturnType): SlantGraph // Follows the store only while the graph has subscribers: a graph nobody subscribes to (e.g. after unmount, or from a // render that never committed) does not stay subscribed to the store. const subscribeToGraph = graph.subscribe.bind(graph); + const getGraphState = graph.getState.bind(graph); const subscribers = new Set(); let unsubscribeFromStore: (() => void) | undefined; + // Catches up with changes made while the graph has no subscribers, e.g. when a hidden (React 19.2+) ran the + // cleanup of effects and renders before subscribing again. Returns the same state until the store activities change. + graph.getState = () => { + unsubscribeFromStore || transacting || handleStoreChange(); + + return getGraphState(); + }; + + // Catches up before a transaction, so its middleware sees the activities, e.g. to link nodes to them. + graph.act = fn => { + unsubscribeFromStore || handleStoreChange(); + transact(fn); + }; + graph.subscribe = subscriber => { const unsubscribe = subscribeToGraph(subscriber);