Skip to content

Commit ca9befe

Browse files
committed
fix edge case with duplicate nodes on host side
1 parent 3aa9bd1 commit ca9befe

11 files changed

Lines changed: 80 additions & 40 deletions

File tree

e2e/mutations.e2e.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import {expect, test} from '@playwright/test';
1+
import { expect, test } from '@playwright/test';
22

33
const testSet: Array<[string, string, string]> = [
44
['iframe', 'react-mutations-1', 'Data: 1\nData: 2\nData: 3\nData: 4'],
5-
['iframe', 'react-mutations-2', 'Data: 1\nData: 2'],
5+
['iframe', 'react-mutations-2', 'Data: 1\nData: 2\nData: 3\nData: 4'],
6+
['iframe', 'react-mutations-3', 'Data: 1\nData: 2'],
67
];
78

89
testSet.forEach(([sandbox, example, expectedText]) => {

examples/kitchen-sink/app/host/components.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ export function ControlPanel({
164164
<option value="react-remote-ui">React Remote UI</option>
165165
<option value="react-mutations-1">React Mutations 1</option>
166166
<option value="react-mutations-2">React Mutations 2</option>
167+
<option value="react-mutations-3">React Mutations 3</option>
167168
</Select>
168169
</section>
169170

@@ -200,6 +201,7 @@ const EXAMPLE_FILE_NAMES = new Map<RenderExample, string>([
200201
['react', 'react.tsx'],
201202
['react-mutations-1', 'react-mutations.tsx'],
202203
['react-mutations-2', 'react-mutations.tsx'],
204+
['react-mutations-3', 'react-mutations.tsx'],
203205
['svelte', 'App.svelte'],
204206
['vue', 'App.vue'],
205207
]);

examples/kitchen-sink/app/host/state.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const ALLOWED_EXAMPLE_VALUES = new Set<RenderExample>([
1717
'vue',
1818
'react-mutations-1',
1919
'react-mutations-2',
20+
'react-mutations-3',
2021
]);
2122

2223
export function createState(

examples/kitchen-sink/app/remote/examples/react-mutations.tsx

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
/** @jsxRuntime automatic */
22
/** @jsxImportSource react */
33

4-
import {createRemoteComponent} from '@remote-dom/react';
5-
import {createRoot} from 'react-dom/client';
4+
import { createRemoteComponent } from '@remote-dom/react';
5+
import { createRoot } from 'react-dom/client';
66

7-
import type {RenderAPI} from '../../types.ts';
8-
import {Stack as StackElement, Text as TextElement} from '../elements.ts';
9-
import {useRenders} from './utils/react-hooks.ts';
7+
import { useEffect, useState } from 'react';
8+
import type { RenderAPI } from '../../types.ts';
9+
import { Stack as StackElement, Text as TextElement } from '../elements.ts';
10+
import { useRenders } from './utils/react-hooks.ts';
1011

1112
const Stack = createRemoteComponent('ui-stack', StackElement);
1213
const Text = createRemoteComponent('ui-text', TextElement);
@@ -48,6 +49,39 @@ const Example1 = () => {
4849
};
4950

5051
const Example2 = () => {
52+
const [loading, setLoading] = useState(true);
53+
54+
useEffect(() => {
55+
setLoading(false);
56+
}, [setLoading]);
57+
58+
return (
59+
<Stack spacing testId="test-stack">
60+
<>
61+
{loading && loading1}
62+
{!loading && (
63+
<>
64+
{data1}
65+
{data2}
66+
</>
67+
)}
68+
</>
69+
<>
70+
{loading && loading2}
71+
{!loading && (
72+
<>
73+
{data3}
74+
{data4}
75+
</>
76+
)}
77+
</>
78+
{!loading && done}
79+
</Stack>
80+
);
81+
};
82+
83+
84+
const Example3 = () => {
5185
const renders = useRenders(2);
5286

5387
return (
@@ -60,14 +94,17 @@ const Example2 = () => {
6094
);
6195
};
6296

97+
6398
function App({api}: {api: RenderAPI}) {
64-
if (api.example === 'react-mutations-1') {
65-
return <Example1 />;
66-
}
99+
const {example} = api;
67100

68-
if (api.example === 'react-mutations-2') {
69-
return <Example2 />;
70-
}
101+
return example === 'react-mutations-1' ? (
102+
<Example1 />
103+
) : example === 'react-mutations-2' ? (
104+
<Example2 />
105+
) : example === 'react-mutations-3' ? (
106+
<Example3 />
107+
) : null;
71108
}
72109

73110
export function renderUsingReact(root: Element, api: RenderAPI) {

examples/kitchen-sink/app/remote/render.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ export async function render(root: Element, api: RenderAPI) {
2323
return renderUsingReact(root, api);
2424
}
2525
case 'react-mutations-1':
26-
case 'react-mutations-2': {
26+
case 'react-mutations-2':
27+
case 'react-mutations-3': {
2728
const {renderUsingReact} = await import('./examples/react-mutations.tsx');
2829
return renderUsingReact(root, api);
2930
}

examples/kitchen-sink/app/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ export type RenderExample =
3030
| 'vue'
3131
| 'react-remote-ui'
3232
| 'react-mutations-1'
33-
| 'react-mutations-2';
33+
| 'react-mutations-2'
34+
| 'react-mutations-3';
3435

3536
/**
3637
* The object that the “host” page will pass to the “remote” environment. This

packages/compat/source/adapter/host.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ export function adaptToLegacyRemoteChannel(
8787
}
8888

8989
const siblings = tree.get(parentId)!;
90+
91+
if (siblings.some((existing) => existing.id === node.id)) {
92+
return;
93+
}
94+
9095
const index =
9196
nextSiblingId === undefined
9297
? siblings.length

packages/core/source/elements/RemoteMutationObserver.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import {
3434
export class RemoteMutationObserver extends MutationObserver {
3535
constructor(private readonly connection: RemoteConnection) {
3636
super((records) => {
37-
const addedNodes: Node[] = [];
3837
const remoteRecords: RemoteMutationRecord[] = [];
3938

4039
for (const record of records) {
@@ -49,25 +48,9 @@ export class RemoteMutationObserver extends MutationObserver {
4948
targetId,
5049
remoteId(node),
5150
]);
52-
53-
addedNodes.splice(addedNodes.indexOf(node), 1);
5451
});
5552

5653
record.addedNodes.forEach((node) => {
57-
if (
58-
addedNodes.some(
59-
(addedNode) => addedNode === node || addedNode.contains(node),
60-
)
61-
) {
62-
// A mutation observer will queue some changes, so we might get one record
63-
// for attaching a parent element, and additional records for attaching descendants.
64-
// We serialize the entire tree when a new node was added, so we don’t want to
65-
// send additional “insert child” records when we see those descendants — they
66-
// will already be included the insertion of the parent.
67-
return;
68-
}
69-
70-
addedNodes.push(node);
7154
connectRemoteNode(node, connection);
7255

7356
remoteRecords.push([

packages/core/source/receivers/DOMRemoteReceiver.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ export class DOMRemoteReceiver {
110110
parentId === ROOT_ID ? this.root : attached.get(parentId)!;
111111
const normalizedChild = attach(child);
112112

113+
if (parent.contains(normalizedChild)) {
114+
return;
115+
}
116+
113117
const existingTimeout = destroyTimeouts.get(parentId);
114118
if (existingTimeout) clearTimeout(existingTimeout);
115119

packages/core/source/receivers/RemoteReceiver.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ export class RemoteReceiver {
165165
insertChild: (parentId, child, nextSiblingId) => {
166166
const parent = attached.get(parentId) as Writable<RemoteReceiverParent>;
167167
const children = parent.children as Writable<RemoteReceiverNode[]>;
168+
169+
if (children.some((existing) => existing.id === child.id)) {
170+
return;
171+
}
172+
168173
const normalizedChild = attach(child, parent);
169174

170175
if (nextSiblingId === undefined) {

0 commit comments

Comments
 (0)