Skip to content

Commit 9427808

Browse files
committed
fix(yield-wrapper): prevent circular reference in node replacement
1 parent 412373a commit 9427808

File tree

6 files changed

+149
-1
lines changed

6 files changed

+149
-1
lines changed

react-migration-toolkit/src/components/yield-wrapper.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ export const YieldWrapper: React.FC<YieldWrapperProps> = ({ nodes }) => {
1212

1313
if (element?.parentNode) {
1414
const fragment = document.createDocumentFragment();
15-
for (const node of nodes) {
15+
16+
// Filter out the wrapper element itself from the nodes to prevent circular reference
17+
const filteredNodes = nodes.filter((node) => node !== element);
18+
19+
for (const node of filteredNodes) {
1620
fragment.appendChild(node);
1721
}
1822

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
export { Link } from './link';
22
export { NavLink } from './nav-link';
3+
export { YieldWrapper } from './yield-wrapper';
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { useEffect, useRef } from 'react';
2+
3+
export interface YieldWrapperProps {
4+
nodes: ChildNode[];
5+
}
6+
7+
export const YieldWrapper: React.FC<YieldWrapperProps> = ({ nodes }) => {
8+
const elRef = useRef<HTMLSpanElement | null>(null);
9+
10+
useEffect(() => {
11+
const element = elRef.current;
12+
13+
if (element?.parentNode) {
14+
const fragment = document.createDocumentFragment();
15+
16+
// Filter out the wrapper element itself from the nodes to prevent circular reference
17+
const filteredNodes = nodes.filter((node) => node !== element);
18+
19+
for (const node of filteredNodes) {
20+
fragment.appendChild(node);
21+
}
22+
23+
// Replace <span> by fragment with appended nodes
24+
element.parentNode.replaceChild(fragment, element);
25+
}
26+
}, [nodes]);
27+
28+
// This element is temporary. When mounted, it is replaced by the children nodes received from Ember.
29+
return <span ref={elRef}></span>;
30+
};
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import React from "react";
2+
import { YieldWrapper } from "@qonto/react-migration-toolkit/react/components";
3+
4+
interface YieldWrapperExampleProps {
5+
nodes: ChildNode[];
6+
}
7+
8+
export const YieldWrapperExample: React.FC<YieldWrapperExampleProps> = ({
9+
nodes,
10+
}) => {
11+
return <YieldWrapper nodes={nodes} />;
12+
};
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import React from "react";
2+
import { YieldWrapper } from "@qonto/react-migration-toolkit/react/components";
3+
4+
interface YieldWrapperExampleProps {
5+
// eslint-disable-next-line no-undef
6+
nodes: ChildNode[];
7+
}
8+
9+
export const YieldWrapperExample: React.FC<YieldWrapperExampleProps> = ({
10+
nodes,
11+
}) => {
12+
return <YieldWrapper nodes={nodes} />;
13+
};
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { module, test } from 'qunit';
2+
import { setupRenderingTest } from 'ember-qunit';
3+
import { render, settled } from '@ember/test-helpers';
4+
import { hbs } from 'ember-cli-htmlbars';
5+
import { YieldWrapperExample } from 'test-app/react/yield-wrapper-example';
6+
7+
module('Integration | Component | yield-wrapper', function (hooks) {
8+
setupRenderingTest(hooks);
9+
10+
hooks.beforeEach(function () {
11+
this.setProperties({
12+
YieldWrapperExample,
13+
});
14+
});
15+
16+
test('it correctly handles yielded content through React bridge', async function (assert) {
17+
// Create test nodes that would be yielded from an Ember component
18+
const textNode = document.createTextNode('Hello World');
19+
const divNode = document.createElement('div');
20+
divNode.textContent = 'Test Div';
21+
divNode.setAttribute('data-test-id', 'test-div');
22+
23+
const nodes = [textNode, divNode];
24+
25+
// Render using ReactBridge to simulate real usage
26+
this.set('nodes', nodes);
27+
await render(hbs`
28+
<ReactBridge
29+
@reactComponent={{this.YieldWrapperExample}}
30+
@props={{hash nodes=this.nodes}}
31+
/>
32+
`);
33+
34+
// Initially, there should be a span element
35+
assert.dom('span').exists('Wrapper span should exist initially');
36+
37+
// Wait for the next tick to allow the useEffect to run
38+
await settled();
39+
40+
// The span should be replaced with our nodes
41+
assert.dom('span').doesNotExist('Wrapper span should be removed');
42+
assert.dom().hasText('Hello World', 'Text node should be rendered');
43+
assert
44+
.dom('[data-test-id="test-div"]')
45+
.hasText('Test Div', 'Div node should be rendered');
46+
});
47+
48+
test('it handles empty nodes array through React bridge', async function (assert) {
49+
this.set('nodes', []);
50+
await render(hbs`
51+
<ReactBridge
52+
@reactComponent={{this.YieldWrapperExample}}
53+
@props={{hash nodes=this.nodes}}
54+
/>
55+
`);
56+
57+
await settled();
58+
59+
assert.dom('span').doesNotExist('Wrapper span should be removed');
60+
assert.dom().hasText('', 'Container should be empty');
61+
});
62+
63+
test('it handles nested elements through React bridge', async function (assert) {
64+
const parentDiv = document.createElement('div');
65+
parentDiv.setAttribute('data-test-id', 'parent');
66+
const childSpan = document.createElement('span');
67+
childSpan.textContent = 'Nested Content';
68+
childSpan.setAttribute('data-test-id', 'child');
69+
parentDiv.appendChild(childSpan);
70+
71+
const nodes = [parentDiv];
72+
73+
this.set('nodes', nodes);
74+
await render(hbs`
75+
<ReactBridge
76+
@reactComponent={{this.YieldWrapperExample}}
77+
@props={{hash nodes=this.nodes}}
78+
/>
79+
`);
80+
81+
await settled();
82+
83+
assert.dom('span').doesNotExist('Wrapper span should be removed');
84+
assert
85+
.dom('[data-test-id="parent"] [data-test-id="child"]')
86+
.hasText('Nested Content', 'Nested content should be rendered');
87+
});
88+
});

0 commit comments

Comments
 (0)