Skip to content
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

[ServerRenderer] Add option to send instructions as data attributes #25437

Merged
merged 13 commits into from
Nov 30, 2022
Merged

[ServerRenderer] Add option to send instructions as data attributes #25437

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fe5caba
Add renames
mofeiZ Oct 31, 2022
5e81c7d
Add non-execution format, runtime, and unit tests
mofeiZ Nov 1, 2022
575c581
Add EmbeddingContext enum type
mofeiZ Nov 2, 2022
62bf65f
Add external runtime to new tests from rebase
mofeiZ Nov 3, 2022
7daf109
Remove instruction divs from DOM after processing
mofeiZ Nov 3, 2022
7920bb2
changed data attribute names, div -> template
mofeiZ Nov 7, 2022
7e14a26
Feedback: renamed data attributes, check readyState, make tests explicit
mofeiZ Nov 17, 2022
a3ee1a7
Feedback: only watch direct children of document.body
mofeiZ Nov 17, 2022
4cf4af1
Feedback: fork writeStyleResource for different contexts
mofeiZ Nov 17, 2022
ea37d85
syntax
mofeiZ Nov 18, 2022
df858fe
feedback: listen for document.body, style, naming
mofeiZ Nov 29, 2022
80a3b3d
changed some comments
mofeiZ Nov 29, 2022
f35926f
feedback: actually call observe
mofeiZ Nov 30, 2022
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
14 changes: 12 additions & 2 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 @@ -275,7 +275,7 @@ type PreinitOptions = {
crossOrigin?: string,
integrity?: string,
};
function preinit(href: string, options: PreinitOptions) {
function preinit(href: string, options: PreinitOptions): void {
if (!currentResources) {
// While we expect that preinit calls are primarily going to be observed
// during render because effects and events don't run on the server it is
Expand All @@ -285,7 +285,17 @@ function preinit(href: string, options: PreinitOptions) {
// simply return and do not warn.
return;
}
const resources = currentResources;
preinitImpl(currentResources, href, options);
}

// On the server, preinit may be called outside of render when sending an
// external SSR runtime as part of the initial resources payload. Since this
// is an internal React call, we do not need to use the resources stack.
export function preinitImpl(
resources: Resources,
href: string,
options: PreinitOptions,
): void {
if (__DEV__) {
validatePreinitArguments(href, options);
}
Expand Down
99 changes: 89 additions & 10 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 @@ -3,6 +3,7 @@
* clients. Therefore, it should be fast and not have many external dependencies.
* @flow
*/
/* eslint-disable dot-notation */

// Imports are resolved statically by the closure compiler in release bundles
// and by rollup in jest unit tests
Expand All @@ -13,13 +14,91 @@ import {
completeSegment,
} from './fizz-instruction-set/ReactDOMFizzInstructionSet';

// Intentionally does nothing. Implementation will be added in future PR.
// eslint-disable-next-line no-unused-vars
const observer = new MutationObserver(mutations => {
// These are only called so I can check what the module output looks like. The
// code is unreachable.
clientRenderBoundary();
completeBoundaryWithStyles();
completeBoundary();
completeSegment();
});
if (!window.$RC) {
// TODO: Eventually remove, we currently need to set these globals for
// compatibility with ReactDOMFizzInstructionSet
window.$RC = completeBoundary;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why expose this on the window? We shouldn't need to expose anything on the global when we're running a custom external runtime with its own closure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we currently access this variable through window here.

We can add a compile-time variable (similar to __DEV__) to compile this access away when bundling the external runtime. There probably is a better solution I'm missing though

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I left a TODO for that when I set up the Closure compilation step. I tried setting it up so that the arguments get passed into the function ("dependency injection"), which worked, but Closure wasn't inlining it as well as it should in the inline runtime. I think it could be solved by running Rollup before running Closure, like we do for our main bundles.

I can look into improving that if you like @mofeiZ

window.$RM = new Map();
}

if (document.readyState === 'loading') {
if (document.body != null) {
installFizzInstrObserver(document.body);
} else {
// body may not exist yet if the fizz runtime is sent in <head>
// (e.g. as a preinit resource)
const domBodyObserver = new MutationObserver(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like I am missing something but does this observer ever observe anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh I can't believe I didn't call observe, thanks so much for catching this - definitely a problem.

// We expect the body node to be stable once parsed / created
if (document.body) {
if (document.readyState === 'loading') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure the order of DOM parsing / changing readyState - this could totally be a redundant check

Added just in case readyState gets set loading -> interactive before mutation observer listeners are triggered.

(experimented with inline scripts on my laptop's version of Chrome, but wary about other browser implementations)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know off the top of my head. seems safe enough to leave in

installFizzInstrObserver(document.body);
}
handleExistingNodes();
domBodyObserver.disconnect();
}
});
}
}

handleExistingNodes();

function handleExistingNodes() {
const existingNodes = document.getElementsByTagName('template');
for (let i = 0; i < existingNodes.length; i++) {
handleNode(existingNodes[i]);
}
}

function installFizzInstrObserver(target /*: Node */) {
const fizzInstrObserver = new MutationObserver(mutations => {
for (let i = 0; i < mutations.length; i++) {
const addedNodes = mutations[i].addedNodes;
for (let j = 0; j < addedNodes.length; j++) {
if (addedNodes.item(j).parentNode) {
handleNode(addedNodes.item(j));
}
}
}
});
// We assume that instruction data nodes are eventually appended to the
// body, even if Fizz is streaming to a shell / subtree.
fizzInstrObserver.observe(target, {
childList: true,
});
window.addEventListener('DOMContentLoaded', () => {
fizzInstrObserver.disconnect();
});
}

function handleNode(node_ /*: Node */) {
// $FlowFixMe[incompatible-cast]
if (node_.nodeType !== 1 || !(node_ /*: HTMLElement*/).dataset) {
return;
}
// $FlowFixMe[incompatible-cast]
const node = (node_ /*: HTMLElement*/);
const dataset = node.dataset;
if (dataset['rxi'] != null) {
clientRenderBoundary(
dataset['bid'],
dataset['dgst'],
dataset['msg'],
dataset['stck'],
);
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']),
);
node.remove();
} else if (dataset['rci'] != null) {
completeBoundary(dataset['bid'], dataset['sid']);
node.remove();
} else if (dataset['rsi'] != null) {
completeSegment(dataset['sid'], dataset['pid']);
node.remove();
}
}
Loading