Skip to content

[Fizz] implement renderIntoContainer #25703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

[Fizz] implement renderIntoContainer #25703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
90d4c87
implement renderIntoContainer
gnoff Nov 18, 2022
d19b102
only expose renderIntoContainer* when feature flag is on
gnoff Jan 12, 2023
0c2c4a3
refactor external runtime instruction execution
gnoff Jan 8, 2023
8be28e5
refactor inline runtime for style insertion
gnoff Jan 8, 2023
f472e4b
refactor root boundary ID out of responseState and eliminate forking …
gnoff Jan 17, 2023
d6002cf
update export to statically exclude renderIntoContainer in stable build
gnoff Jan 17, 2023
3273db7
revert change to ReactVersion
gnoff Jan 17, 2023
9bad42a
reduce stack assignment
gnoff Jan 17, 2023
8e5cd60
refactor external runtime instructions for density
gnoff Jan 17, 2023
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
1 change: 1 addition & 0 deletions packages/react-dom-bindings/src/server/ReactDOMFloatServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ type BaseResource = {
flushed: boolean,
};

export type LinkTagResource = PreloadResource | StyleResource | LinkResource;
export type Resource = PreloadResource | StyleResource | ScriptResource;
export type HeadResource =
| TitleResource
Expand Down
52 changes: 30 additions & 22 deletions packages/react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,12 @@
import {
clientRenderBoundary,
completeBoundaryWithStyles,
completeContainerWithStyles,
completeBoundary,
completeContainer,
completeSegment,
} from './fizz-instruction-set/ReactDOMFizzInstructionSetExternalRuntime';

if (!window.$RC) {
// TODO: Eventually remove, we currently need to set these globals for
// compatibility with ReactDOMFizzInstructionSet
window.$RC = completeBoundary;
window.$RM = new Map();
}

if (document.readyState === 'loading') {
if (document.body != null) {
installFizzInstrObserver(document.body);
Expand Down Expand Up @@ -74,34 +69,47 @@ function installFizzInstrObserver(target /*: Node */) {
}

function handleNode(node_ /*: Node */) {
// $FlowFixMe[incompatible-cast]
if (node_.nodeType !== 1 || !(node_ /*: HTMLElement*/).dataset) {
if (
node_.nodeType !== 1 ||
// $FlowFixMe[incompatible-cast]
!(node_ /*: HTMLElement*/).dataset
) {
return;
}
// $FlowFixMe[incompatible-cast]
const node = (node_ /*: HTMLElement*/);
const dataset = node.dataset;
if (dataset['rxi'] != null) {
clientRenderBoundary(
dataset['bid'],

if (dataset['ix'] != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question - would we expect / handle any of these instruction functions to throw here? I noticed a try-catch in completeContainer, but couldn't find what might throw in that function. Other instruction functions don't look like they would throw either

We should add a try-catch here if other functions can potentially throw (and React is expected to keep rendering). Inline scripts isolate failures, but the external runtime may skip processing further instructions / MutationRecords if one errors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I ended up removing the throw but not the wrapping try catch. Good call on handling it here though, we probably should not actually throw and just return from completeContainer

node.remove();
return clientRenderBoundary(
dataset['ix'],
dataset['dgst'],
dataset['msg'],
dataset['stck'],
);
} else if (dataset['is'] != null) {
node.remove();
return completeSegment(dataset['is'], dataset['p']);
} else if (dataset['ir'] != null) {
node.remove();
} else if (dataset['rri'] != null) {
// Convert styles here, since its type is Array<Array<string>>
completeBoundaryWithStyles(
dataset['bid'],
dataset['sid'],
JSON.parse(dataset['sty']),
return completeBoundaryWithStyles(
dataset['ir'],
dataset['s'],
JSON.parse(dataset['r']),
);
} else if (dataset['ib'] != null) {
node.remove();
} else if (dataset['rci'] != null) {
completeBoundary(dataset['bid'], dataset['sid']);
return completeBoundary(dataset['ib'], dataset['s']);
} else if (dataset['ik'] != null) {
node.remove();
} else if (dataset['rsi'] != null) {
completeSegment(dataset['sid'], dataset['pid']);
return completeContainerWithStyles(
dataset['ik'],
dataset['s'],
JSON.parse(dataset['r']),
);
} else if (dataset['ic'] != null) {
node.remove();
return completeContainer(dataset['ic'], dataset['s']);
}
}
Loading