Skip to content

Commit 04bd67a

Browse files
authored
Resolve references to deduped owner objects (facebook#30549)
This is a follow-up from facebook#30528 to not only handle props (the critical change), but also the owner ~and stack~ of a referenced element. ~Handling stacks here is rather academic because the Flight Server currently does not deduplicate owner stacks. And if they are really identical, we should probably just dedupe the whole element.~ EDIT: Removed from the PR. Handling owner objects on the other hand is an actual requirement as reported in vercel/next.js#69545. This problem only affects the stable release channel, as the absence of owner stacks allows for the specific kind of shared owner deduping as demonstrated in the unit test.
1 parent 4708fb9 commit 04bd67a

File tree

3 files changed

+134
-9
lines changed

3 files changed

+134
-9
lines changed

packages/react-client/src/ReactFlightClient.js

+18-9
Original file line numberDiff line numberDiff line change
@@ -937,26 +937,35 @@ function waitForReference<T>(
937937
}
938938
value = value[path[i]];
939939
}
940-
parentObject[key] = map(response, value);
940+
const mappedValue = map(response, value);
941+
parentObject[key] = mappedValue;
941942

942943
// If this is the root object for a model reference, where `handler.value`
943944
// is a stale `null`, the resolved value can be used directly.
944945
if (key === '' && handler.value === null) {
945-
handler.value = parentObject[key];
946+
handler.value = mappedValue;
946947
}
947948

948-
// If the parent object is an unparsed React element tuple and its outlined
949-
// props have now been resolved, we also need to update the props of the
950-
// parsed element object (i.e. handler.value).
949+
// If the parent object is an unparsed React element tuple, we also need to
950+
// update the props and owner of the parsed element object (i.e.
951+
// handler.value).
951952
if (
952953
parentObject[0] === REACT_ELEMENT_TYPE &&
953-
key === '3' &&
954954
typeof handler.value === 'object' &&
955955
handler.value !== null &&
956-
handler.value.$$typeof === REACT_ELEMENT_TYPE &&
957-
handler.value.props === null
956+
handler.value.$$typeof === REACT_ELEMENT_TYPE
958957
) {
959-
handler.value.props = parentObject[key];
958+
const element: any = handler.value;
959+
switch (key) {
960+
case '3':
961+
element.props = mappedValue;
962+
break;
963+
case '4':
964+
if (__DEV__) {
965+
element._owner = mappedValue;
966+
}
967+
break;
968+
}
960969
}
961970

962971
handler.deps--;

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js

+113
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,119 @@ describe('ReactFlightDOMBrowser', () => {
630630
expect(container.innerHTML).toBe('<div></div>');
631631
});
632632

633+
it('should handle references to deduped owner objects', async () => {
634+
// This is replicating React components as generated by @svgr/webpack:
635+
let path1a: React.ReactNode;
636+
let path1b: React.ReactNode;
637+
let path2: React.ReactNode;
638+
639+
function Svg1() {
640+
return ReactServer.createElement(
641+
'svg',
642+
{id: '1'},
643+
path1a || (path1a = ReactServer.createElement('path', {})),
644+
path1b || (path1b = ReactServer.createElement('path', {})),
645+
);
646+
}
647+
648+
function Svg2() {
649+
return ReactServer.createElement(
650+
'svg',
651+
{id: '2'},
652+
path2 || (path2 = ReactServer.createElement('path', {})),
653+
);
654+
}
655+
656+
function Server() {
657+
return ReactServer.createElement(
658+
ReactServer.Fragment,
659+
{},
660+
ReactServer.createElement(Svg1),
661+
ReactServer.createElement(Svg2),
662+
);
663+
}
664+
665+
let stream = await serverAct(() =>
666+
ReactServerDOMServer.renderToReadableStream(<Server />, webpackMap),
667+
);
668+
669+
function ClientRoot({response}) {
670+
return use(response);
671+
}
672+
673+
let response = ReactServerDOMClient.createFromReadableStream(stream);
674+
const container = document.createElement('div');
675+
const root = ReactDOMClient.createRoot(container);
676+
677+
await act(() => {
678+
root.render(<ClientRoot response={response} />);
679+
});
680+
681+
const expectedHtml =
682+
'<svg id="1"><path></path><path></path></svg><svg id="2"><path></path></svg>';
683+
684+
expect(container.innerHTML).toBe(expectedHtml);
685+
686+
// Render a second time:
687+
688+
// Assigning the path elements to variables in module scope (here simulated
689+
// with the test's function scope), and rendering a second time, prevents
690+
// the owner of the path elements (i.e. Svg1/Svg2) to be deduped. The owner
691+
// of the path in Svg1 is fully inlined. The owner of the owner of the path
692+
// in Svg2 is Server, which is deduped and replaced with a reference to the
693+
// owner of the owner of the path in Svg1. This nested owner is actually
694+
// Server from the previous render pass, which is kinda broken and libraries
695+
// probably shouldn't generate code like this. This reference can only be
696+
// resolved properly if owners are specifically handled when resolving
697+
// outlined models.
698+
699+
stream = await serverAct(() =>
700+
ReactServerDOMServer.renderToReadableStream(<Server />, webpackMap),
701+
);
702+
703+
response = ReactServerDOMClient.createFromReadableStream(stream);
704+
705+
await act(() => {
706+
root.render(<ClientRoot response={response} />);
707+
});
708+
709+
expect(container.innerHTML).toBe(expectedHtml);
710+
711+
if (__DEV__) {
712+
const resolvedPath1b = await response.value[0].props.children[1]._payload;
713+
714+
expect(resolvedPath1b._owner).toEqual(
715+
expect.objectContaining({
716+
name: 'Svg1',
717+
env: 'Server',
718+
key: null,
719+
owner: expect.objectContaining({
720+
name: 'Server',
721+
env: 'Server',
722+
key: null,
723+
owner: null,
724+
}),
725+
}),
726+
);
727+
728+
const resolvedPath2 = response.value[1].props.children;
729+
730+
expect(resolvedPath2._owner).toEqual(
731+
expect.objectContaining({
732+
name: 'Svg2',
733+
env: 'Server',
734+
key: null,
735+
owner: expect.objectContaining({
736+
name: 'Server',
737+
env: 'Server',
738+
key: null,
739+
owner: null,
740+
}),
741+
}),
742+
);
743+
}
744+
});
745+
633746
it('should progressively reveal server components', async () => {
634747
let reportedErrors = [];
635748

packages/react-server/src/ReactFlightServer.js

+3
Original file line numberDiff line numberDiff line change
@@ -2665,6 +2665,9 @@ function renderModelDestructive(
26652665
case '3':
26662666
propertyName = 'props';
26672667
break;
2668+
case '4':
2669+
propertyName = '_owner';
2670+
break;
26682671
}
26692672
}
26702673
writtenObjects.set(value, parentReference + ':' + propertyName);

0 commit comments

Comments
 (0)