Skip to content

Commit

Permalink
[Fizz][Float] <img> inside <picture> should not preload during SSR (
Browse files Browse the repository at this point in the history
facebook#27346)

img tags inside picture tags should not automatically be preloaded
because usually the img is a fallback. We will consider a more
comprehensive way of preloading picture tags which may require a
technique like using an inline script to construct the image in the
browser but for now we simply omit the preloads to avoid harming load
times by loading fallbacks.
  • Loading branch information
gnoff authored and AndyPengc12 committed Apr 15, 2024
1 parent a364e60 commit 2c3ae5e
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 40 deletions.
81 changes: 41 additions & 40 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -499,22 +499,26 @@ const HTML_COLGROUP_MODE = 8;

type InsertionMode = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8;

const NO_SCOPE = /* */ 0b00;
const NOSCRIPT_SCOPE = /* */ 0b01;
const PICTURE_SCOPE = /* */ 0b10;

// Lets us keep track of contextual state and pick it back up after suspending.
export type FormatContext = {
insertionMode: InsertionMode, // root/svg/html/mathml/table
selectedValue: null | string | Array<string>, // the selected value(s) inside a <select>, or null outside <select>
noscriptTagInScope: boolean,
tagScope: number,
};

function createFormatContext(
insertionMode: InsertionMode,
selectedValue: null | string,
noscriptTagInScope: boolean,
tagScope: number,
): FormatContext {
return {
insertionMode,
selectedValue,
noscriptTagInScope,
tagScope,
};
}

Expand All @@ -525,7 +529,7 @@ export function createRootFormatContext(namespaceURI?: string): FormatContext {
: namespaceURI === 'http://www.w3.org/1998/Math/MathML'
? MATHML_MODE
: ROOT_HTML_MODE;
return createFormatContext(insertionMode, null, false);
return createFormatContext(insertionMode, null, NO_SCOPE);
}

export function getChildFormatContext(
Expand All @@ -535,80 +539,70 @@ export function getChildFormatContext(
): FormatContext {
switch (type) {
case 'noscript':
return createFormatContext(HTML_MODE, null, true);
return createFormatContext(
HTML_MODE,
null,
parentContext.tagScope | NOSCRIPT_SCOPE,
);
case 'select':
return createFormatContext(
HTML_MODE,
props.value != null ? props.value : props.defaultValue,
parentContext.noscriptTagInScope,
parentContext.tagScope,
);
case 'svg':
return createFormatContext(SVG_MODE, null, parentContext.tagScope);
case 'picture':
return createFormatContext(
SVG_MODE,
HTML_MODE,
null,
parentContext.noscriptTagInScope,
parentContext.tagScope | PICTURE_SCOPE,
);
case 'math':
return createFormatContext(
MATHML_MODE,
null,
parentContext.noscriptTagInScope,
);
return createFormatContext(MATHML_MODE, null, parentContext.tagScope);
case 'foreignObject':
return createFormatContext(
HTML_MODE,
null,
parentContext.noscriptTagInScope,
);
return createFormatContext(HTML_MODE, null, parentContext.tagScope);
// Table parents are special in that their children can only be created at all if they're
// wrapped in a table parent. So we need to encode that we're entering this mode.
case 'table':
return createFormatContext(
HTML_TABLE_MODE,
null,
parentContext.noscriptTagInScope,
);
return createFormatContext(HTML_TABLE_MODE, null, parentContext.tagScope);
case 'thead':
case 'tbody':
case 'tfoot':
return createFormatContext(
HTML_TABLE_BODY_MODE,
null,
parentContext.noscriptTagInScope,
parentContext.tagScope,
);
case 'colgroup':
return createFormatContext(
HTML_COLGROUP_MODE,
null,
parentContext.noscriptTagInScope,
parentContext.tagScope,
);
case 'tr':
return createFormatContext(
HTML_TABLE_ROW_MODE,
null,
parentContext.noscriptTagInScope,
parentContext.tagScope,
);
}
if (parentContext.insertionMode >= HTML_TABLE_MODE) {
// Whatever tag this was, it wasn't a table parent or other special parent, so we must have
// entered plain HTML again.
return createFormatContext(
HTML_MODE,
null,
parentContext.noscriptTagInScope,
);
return createFormatContext(HTML_MODE, null, parentContext.tagScope);
}
if (parentContext.insertionMode === ROOT_HTML_MODE) {
if (type === 'html') {
// We've emitted the root and is now in <html> mode.
return createFormatContext(HTML_HTML_MODE, null, false);
return createFormatContext(HTML_HTML_MODE, null, parentContext.tagScope);
} else {
// We've emitted the root and is now in plain HTML mode.
return createFormatContext(HTML_MODE, null, false);
return createFormatContext(HTML_MODE, null, parentContext.tagScope);
}
} else if (parentContext.insertionMode === HTML_HTML_MODE) {
// We've emitted the document element and is now in plain HTML mode.
return createFormatContext(HTML_MODE, null, false);
return createFormatContext(HTML_MODE, null, parentContext.tagScope);
}
return parentContext;
}
Expand Down Expand Up @@ -2457,12 +2451,14 @@ function pushImg(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
resumableState: ResumableState,
pictureTagInScope: boolean,
): null {
const {src, srcSet} = props;
if (
props.loading !== 'lazy' &&
(typeof src === 'string' || typeof srcSet === 'string') &&
props.fetchPriority !== 'low' &&
pictureTagInScope === false &&
// We exclude data URIs in src and srcSet since these should not be preloaded
!(
typeof src === 'string' &&
Expand Down Expand Up @@ -3230,7 +3226,7 @@ export function pushStartInstance(
props,
renderState,
formatContext.insertionMode,
formatContext.noscriptTagInScope,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
)
: pushStartTitle(target, props);
case 'link':
Expand All @@ -3241,7 +3237,7 @@ export function pushStartInstance(
renderState,
textEmbedded,
formatContext.insertionMode,
formatContext.noscriptTagInScope,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
);
case 'script':
return enableFloat
Expand All @@ -3251,7 +3247,7 @@ export function pushStartInstance(
resumableState,
textEmbedded,
formatContext.insertionMode,
formatContext.noscriptTagInScope,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
)
: pushStartGenericElement(target, props, type);
case 'style':
Expand All @@ -3262,7 +3258,7 @@ export function pushStartInstance(
renderState,
textEmbedded,
formatContext.insertionMode,
formatContext.noscriptTagInScope,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
);
case 'meta':
return pushMeta(
Expand All @@ -3271,7 +3267,7 @@ export function pushStartInstance(
renderState,
textEmbedded,
formatContext.insertionMode,
formatContext.noscriptTagInScope,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
);
// Newline eating tags
case 'listing':
Expand All @@ -3280,7 +3276,12 @@ export function pushStartInstance(
}
case 'img': {
return enableFloat
? pushImg(target, props, resumableState)
? pushImg(
target,
props,
resumableState,
!!(formatContext.tagScope & PICTURE_SCOPE),
)
: pushSelfClosing(target, props, type);
}
// Omitted close tags
Expand Down
34 changes: 34 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4085,6 +4085,40 @@ body {
);
});

// https://github.com/vercel/next.js/discussions/54799
it('omits preloads when an <img> is inside a <picture>', async () => {
await act(() => {
renderToPipeableStream(
<html>
<body>
<picture>
<img src="foo" />
</picture>
<picture>
<source type="image/webp" srcSet="webpsrc" />
<img src="jpg fallback" />
</picture>
</body>
</html>,
).pipe(writable);
});

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head />
<body>
<picture>
<img src="foo" />
</picture>
<picture>
<source type="image/webp" srcset="webpsrc" />
<img src="jpg fallback" />
</picture>
</body>
</html>,
);
});

describe('ReactDOM.prefetchDNS(href)', () => {
it('creates a dns-prefetch resource when called', async () => {
function App({url}) {
Expand Down

0 comments on commit 2c3ae5e

Please sign in to comment.