Skip to content

Commit

Permalink
Import maps need to be emitted before any scripts or preloads so the …
Browse files Browse the repository at this point in the history
…browser can properly locate these resources. This change makes React aware of the concept of import maps and emits them before scripts and modules and their preloads.
  • Loading branch information
gnoff committed Aug 21, 2023
1 parent b277259 commit 53dcbb2
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 19 deletions.
51 changes: 35 additions & 16 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -1968,7 +1968,7 @@ export function clearSingleton(instance: Instance): void {

export const supportsResources = true;

type HoistableTagType = 'link' | 'meta' | 'title';
type HoistableTagType = 'link' | 'meta' | 'title' | 'script';
type TResource<
T: 'stylesheet' | 'style' | 'script' | 'void',
S: null | {...},
Expand Down Expand Up @@ -2759,7 +2759,12 @@ export function getResource(
return null;
}
case 'script': {
if (typeof pendingProps.src === 'string' && pendingProps.async === true) {
if (pendingProps.type === 'importmap') {
return null;
} else if (
typeof pendingProps.src === 'string' &&
pendingProps.async === true
) {
const scriptProps: ScriptProps = pendingProps;
const key = getScriptKey(scriptProps.src);
const scripts = getResourcesFromRoot(resourceRoot).hoistableScripts;
Expand Down Expand Up @@ -3110,21 +3115,21 @@ export function hydrateHoistable(
case 'title': {
instance = ownerDocument.getElementsByTagName('title')[0];
if (
!instance ||
isOwnedInstance(instance) ||
instance.namespaceURI === SVG_NAMESPACE ||
instance.hasAttribute('itemprop')
instance &&
!isOwnedInstance(instance) &&
instance.namespaceURI !== SVG_NAMESPACE &&
!instance.hasAttribute('itemprop')
) {
instance = ownerDocument.createElement(type);
(ownerDocument.head: any).insertBefore(
instance,
ownerDocument.querySelector('head > title'),
);
setInitialProperties(instance, type, props);
break;
}
instance = ownerDocument.createElement(type);
setInitialProperties(instance, type, props);
precacheFiberNode(internalInstanceHandle, instance);
markNodeAsHoistable(instance);
return instance;
(ownerDocument.head: any).insertBefore(
instance,
ownerDocument.querySelector('head > title'),
);
break;
}
case 'link': {
const cache = getHydratableHoistableCache('link', 'href', ownerDocument);
Expand Down Expand Up @@ -3201,6 +3206,17 @@ export function hydrateHoistable(
(ownerDocument.head: any).appendChild(instance);
break;
}
case 'script': {
// the only hoistable script is type="importmap" and there should only be 1
instance = ownerDocument.querySelector('script[type="importmap"]');
if (instance && !isOwnedInstance(instance)) {
break;
}
instance = ownerDocument.createElement(type);
setInitialProperties(instance, type, props);
(ownerDocument.head: any).appendChild(instance);
break;
}
default:
throw new Error(
`getNodesForType encountered a type it did not expect: "${type}". This is a bug in React.`,
Expand Down Expand Up @@ -3406,7 +3422,9 @@ export function isHostHoistableType(
}
}
case 'script': {
if (
if (props.type === 'importmap') {
return true;
} else if (
props.async !== true ||
props.onLoad ||
props.onError ||
Expand Down Expand Up @@ -3436,8 +3454,9 @@ export function isHostHoistableType(
}
}
return false;
} else {
return true;
}
return true;
}
case 'noscript':
case 'template': {
Expand Down
21 changes: 21 additions & 0 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export type ResponseState = {
// Hoistable chunks
charsetChunks: Array<Chunk | PrecomputedChunk>,
preconnectChunks: Array<Chunk | PrecomputedChunk>,
importMapChunks: Array<Chunk | PrecomputedChunk>,
preloadChunks: Array<Chunk | PrecomputedChunk>,
hoistableChunks: Array<Chunk | PrecomputedChunk>,

Expand Down Expand Up @@ -361,6 +362,7 @@ export function createResponseState(
hasBody: false,
charsetChunks: [],
preconnectChunks: [],
importMapChunks: [],
preloadChunks: [],
hoistableChunks: [],
stylesToHoist: false,
Expand Down Expand Up @@ -2701,12 +2703,17 @@ function pushStartHtml(
function pushScript(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
responseState: ResponseState,
resources: Resources,
textEmbedded: boolean,
insertionMode: InsertionMode,
noscriptTagInScope: boolean,
): null {
if (enableFloat) {
if (props.type === 'importmap') {
return pushScriptImpl(responseState.importMapChunks, props);
}

const asyncProp = props.async;
if (
typeof props.src !== 'string' ||
Expand Down Expand Up @@ -3122,6 +3129,7 @@ export function pushStartInstance(
? pushScript(
target,
props,
responseState,
resources,
textEmbedded,
formatContext.insertionMode,
Expand Down Expand Up @@ -4231,6 +4239,12 @@ export function writePreamble(
// Flush unblocked stylesheets by precedence
resources.precedences.forEach(flushAllStylesInPreamble, destination);

const importMapChunks = responseState.importMapChunks;
for (i = 0; i < importMapChunks.length; i++) {
writeChunk(destination, importMapChunks[i]);
}
importMapChunks.length = 0;

resources.bootstrapScripts.forEach(flushResourceInPreamble, destination);

resources.scripts.forEach(flushResourceInPreamble, destination);
Expand Down Expand Up @@ -4301,6 +4315,13 @@ export function writeHoistables(
// but we want to kick off preloading as soon as possible
resources.precedences.forEach(preloadLateStyles, destination);

// Import maps should have already flushed. If they haven't there is a chance we still haven't emitted any preloads for scripts
// or modules so we will emit them now
const importMapChunks = responseState.importMapChunks;
for (i = 0; i < importMapChunks.length; i++) {
writeChunk(destination, importMapChunks[i]);
}

// bootstrap scripts should flush above script priority but these can only flush in the preamble
// so we elide the code here for performance

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export type ResponseState = {
hasBody: boolean,
charsetChunks: Array<Chunk | PrecomputedChunk>,
preconnectChunks: Array<Chunk | PrecomputedChunk>,
importMapChunks: Array<Chunk | PrecomputedChunk>,
preloadChunks: Array<Chunk | PrecomputedChunk>,
hoistableChunks: Array<Chunk | PrecomputedChunk>,
stylesToHoist: boolean,
Expand Down Expand Up @@ -95,6 +96,7 @@ export function createResponseState(
hasBody: responseState.hasBody,
charsetChunks: responseState.charsetChunks,
preconnectChunks: responseState.preconnectChunks,
importMapChunks: responseState.importMapChunks,
preloadChunks: responseState.preloadChunks,
hoistableChunks: responseState.hoistableChunks,
stylesToHoist: responseState.stylesToHoist,
Expand Down
90 changes: 90 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3999,6 +3999,96 @@ body {
);
});

it('hoists <script type="importmap"> tags above scripts and preloads', async () => {
function App() {
return (
<html>
<body>
hello
<script async={true} src="foo" />
<script type="importmap" data-meaningful="">
first map
</script>
<script type="importmap" data-meaningful="">
second map (two maps do not make sense but we are asserting they
flush anyway)
</script>
<script src="sync script" data-meaningful="" />
</body>
</html>
);
}

await act(() => {
renderToPipeableStream(<App />).pipe(writable);
});

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<script type="importmap" data-meaningful="">
first map
</script>
<script type="importmap" data-meaningful="">
second map (two maps do not make sense but we are asserting they
flush anyway)
</script>
<script async="" src="foo" />
</head>
<body>
hello
<script src="sync script" data-meaningful="" />
</body>
</html>,
);

const root = ReactDOMClient.hydrateRoot(document, <App />);
await waitForAll([]);

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<script type="importmap" data-meaningful="">
first map
</script>
<script type="importmap" data-meaningful="">
second map (two maps do not make sense but we are asserting they
flush anyway)
</script>
<script async="" src="foo" />
{/* The reason we see a second copy of this map is we assume there
will only be one and thus only hydrate the first one we encounter.
All subsequent importmap hoistables will be considered new on the client
This is similar to how <title> hydrates as well */}
<script type="importmap" data-meaningful="">
second map (two maps do not make sense but we are asserting they
flush anyway)
</script>
</head>
<body>
hello
<script src="sync script" data-meaningful="" />
</body>
</html>,
);

root.unmount();

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
{/* This one never hydrated so React leaves it behind */}
<script type="importmap" data-meaningful="">
second map (two maps do not make sense but we are asserting they
flush anyway)
</script>
<script async="" src="foo" />
</head>
<body />
</html>,
);
});

describe('ReactDOM.prefetchDNS(href)', () => {
it('creates a dns-prefetch resource when called', async () => {
function App({url}) {
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/test-utils/FizzTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ async function executeScript(script: Element) {
const newScript = ownerDocument.createElement('script');
newScript.textContent = script.textContent;
// make sure to add nonce back to script if it exists
const scriptNonce = script.getAttribute('nonce');
if (scriptNonce) {
newScript.setAttribute('nonce', scriptNonce);
for (let i = 0; i < script.attributes.length; i++) {
const attribute = script.attributes[i];
newScript.setAttribute(attribute.name, attribute.value);
}

parent.insertBefore(newScript, script);
Expand Down

0 comments on commit 53dcbb2

Please sign in to comment.