Skip to content

Commit

Permalink
restore preinit style flushing behavior and nits
Browse files Browse the repository at this point in the history
  • Loading branch information
@gnoff
gnoff committed Oct 17, 2022
1 parent d6e93c7 commit 843f7c4
Show file tree
Hide file tree
Showing 9 changed files with 334 additions and 377 deletions.
33 changes: 12 additions & 21 deletions packages/react-dom-bindings/src/client/ReactDOMFloatClient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ type ResourceType = 'style' | 'font' | 'script';

type PreloadProps = {
rel: 'preload',
as: ResourceType,
href: string,
[string]: mixed,
};
Expand All @@ -49,7 +48,7 @@ type PreloadResource = {
type StyleProps = {
rel: 'stylesheet',
href: string,
'data-rprec': string,
'data-precedence': string,
[string]: mixed,
};
export type StyleResource = {
Expand Down Expand Up @@ -333,7 +332,7 @@ function stylePropsFromPreinitOptions(
return {
rel: 'stylesheet',
href,
'data-rprec': precedence,
'data-precedence': precedence,
crossOrigin: options.crossOrigin,
};
}
Expand Down Expand Up @@ -363,7 +362,6 @@ type StyleQualifyingProps = {
type PreloadQualifyingProps = {
rel: 'preload',
href: string,
as: ResourceType,
[string]: mixed,
};
type ScriptQualifyingProps = {
Expand Down Expand Up @@ -443,8 +441,8 @@ export function getResource(
if (__DEV__) {
validateLinkPropsForPreloadResource(pendingProps);
}
const {href, as} = pendingProps;
if (typeof href === 'string' && isResourceAsType(as)) {
const {href} = pendingProps;
if (typeof href === 'string') {
// We've asserted all the specific types for PreloadQualifyingProps
const preloadRawProps: PreloadQualifyingProps = (pendingProps: any);
let resource = preloadResources.get(href);
Expand Down Expand Up @@ -539,7 +537,7 @@ function preloadPropsFromRawProps(

function stylePropsFromRawProps(rawProps: StyleQualifyingProps): StyleProps {
const props: StyleProps = Object.assign({}, rawProps);
props['data-rprec'] = rawProps.precedence;
props['data-precedence'] = rawProps.precedence;
props.precedence = null;

return props;
Expand Down Expand Up @@ -804,7 +802,7 @@ function acquireStyleResource(resource: StyleResource): Instance {
props.href,
);
const existingEl = root.querySelector(
`link[rel="stylesheet"][data-rprec][href="${limitedEscapedHref}"]`,
`link[rel="stylesheet"][data-precedence][href="${limitedEscapedHref}"]`,
);
if (existingEl) {
resource.instance = existingEl;
Expand Down Expand Up @@ -937,12 +935,14 @@ function insertStyleInstance(
precedence: string,
root: FloatRoot,
): void {
const nodes = root.querySelectorAll('link[rel="stylesheet"][data-rprec]');
const nodes = root.querySelectorAll(
'link[rel="stylesheet"][data-precedence]',
);
const last = nodes.length ? nodes[nodes.length - 1] : null;
let prior = last;
for (let i = 0; i < nodes.length; i++) {
const node = nodes[i];
const nodePrecedence = node.dataset.rprec;
const nodePrecedence = node.dataset.precedence;
if (nodePrecedence === precedence) {
prior = node;
} else if (prior !== last) {
Expand Down Expand Up @@ -1009,13 +1009,8 @@ export function isHostResourceType(type: string, props: Props): boolean {
);
}
case 'preload': {
const {href, as, onLoad, onError} = props;
return (
!onLoad &&
!onError &&
typeof href === 'string' &&
isResourceAsType(as)
);
const {href, onLoad, onError} = props;
return !onLoad && !onError && typeof href === 'string';
}
}
return false;
Expand All @@ -1030,10 +1025,6 @@ export function isHostResourceType(type: string, props: Props): boolean {
return false;
}

function isResourceAsType(as: mixed): boolean {
return as === 'style' || as === 'font' || as === 'script';
}

// When passing user input into querySelector(All) the embedded string must not alter
// the semantics of the query. This escape function is safe to use when we know the
// provided value is going to be wrapped in double quotes as part of an attribute selector
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom-bindings/src/client/ReactDOMHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -903,15 +903,15 @@ function getNextHydratable(node) {
const rel = linkEl.rel;
if (
rel === 'preload' ||
(rel === 'stylesheet' && linkEl.hasAttribute('data-rprec'))
(rel === 'stylesheet' && linkEl.hasAttribute('data-precedence'))
) {
continue;
}
break;
}
case 'STYLE': {
const styleEl: HTMLStyleElement = (element: any);
if (styleEl.hasAttribute('data-rprec')) {
if (styleEl.hasAttribute('data-precedence')) {
continue;
}
break;
Expand Down Expand Up @@ -942,15 +942,15 @@ function getNextHydratable(node) {
const rel = linkEl.rel;
if (
rel === 'preload' ||
(rel === 'stylesheet' && linkEl.hasAttribute('data-rprec'))
(rel === 'stylesheet' && linkEl.hasAttribute('data-precedence'))
) {
continue;
}
break;
}
case 'STYLE': {
const styleEl: HTMLStyleElement = (element: any);
if (styleEl.hasAttribute('data-rprec')) {
if (styleEl.hasAttribute('data-precedence')) {
continue;
}
break;
Expand Down
36 changes: 14 additions & 22 deletions packages/react-dom-bindings/src/server/ReactDOMFloatServer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type PreloadResource = {
type StyleProps = {
rel: 'stylesheet',
href: string,
'data-rprec': string,
'data-precedence': string,
[string]: mixed,
};
type StyleResource = {
Expand All @@ -52,6 +52,7 @@ type StyleResource = {
flushed: boolean,
inShell: boolean, // flushedInShell
hint: PreloadResource,
set: Set<StyleResource>, // the precedence set this resource should be flushed in
};

type ScriptProps = {
Expand Down Expand Up @@ -236,19 +237,18 @@ function preinit(href: string, options: PreinitOptions) {
const as = options.as;
switch (as) {
case 'style': {
const precedence = options.precedence || 'default';

let resource = resources.stylesMap.get(href);
if (resource) {
if (__DEV__) {
const latestProps = stylePropsFromPreinitOptions(
href,
precedence,
resource.precedence,
options,
);
validateStyleResourceDifference(resource.props, latestProps);
}
} else {
const precedence = options.precedence || 'default';
const resourceProps = stylePropsFromPreinitOptions(
href,
precedence,
Expand All @@ -261,6 +261,7 @@ function preinit(href: string, options: PreinitOptions) {
resourceProps,
);
}
resource.set.add(resource);
resources.explicitStylePreloads.add(resource.hint);

return;
Expand Down Expand Up @@ -380,7 +381,7 @@ function stylePropsFromRawProps(
const props: StyleProps = Object.assign({}, rawProps);
props.href = href;
props.rel = 'stylesheet';
props['data-rprec'] = precedence;
props['data-precedence'] = precedence;
delete props.precedence;

return props;
Expand All @@ -394,7 +395,7 @@ function stylePropsFromPreinitOptions(
return {
rel: 'stylesheet',
href,
'data-rprec': precedence,
'data-precedence': precedence,
crossOrigin: options.crossOrigin,
};
}
Expand All @@ -416,8 +417,10 @@ function createStyleResource(

// If this is the first time we've seen this precedence we encode it's position in our set even though
// we don't add the resource to this set yet
if (!precedences.has(precedence)) {
precedences.set(precedence, new Set());
let precedenceSet = precedences.get(precedence);
if (!precedenceSet) {
precedenceSet = new Set();
precedences.set(precedence, precedenceSet);
}

let hint = preloadsMap.get(href);
Expand Down Expand Up @@ -457,6 +460,7 @@ function createStyleResource(
inShell: false,
props,
hint,
set: precedenceSet,
};
stylesMap.set(href, resource);

Expand Down Expand Up @@ -633,14 +637,7 @@ export function resourcesFromLink(props: Props): boolean {
if (resources.boundaryResources) {
resources.boundaryResources.add(resource);
} else {
// Precedences are constructed eagerly when encountered so this will always exist
// Note that it is important to read the precedence from the resource. It is possible
// that the props precedence is new and does not match the precedence on an earlier
// constructed version of this resource.
const set: Set<StyleResource> = (resources.precedences.get(
resource.precedence,
): any);
set.add(resource);
resource.set.add(resource);
}
return true;
}
Expand Down Expand Up @@ -765,11 +762,6 @@ export function hoistResourcesToRoot(
resources: Resources,
boundaryResources: BoundaryResources,
): void {
const {precedences} = resources;
boundaryResources.forEach(resource => {
// all precedences are set upon discovery. so we know we will have a set here
const set: Set<StyleResource> = (precedences.get(resource.precedence): any);
set.add(resource);
});
boundaryResources.forEach(resource => resource.set.add(resource));
boundaryResources.clear();
}
78 changes: 24 additions & 54 deletions packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2259,7 +2259,7 @@ function escapeJSObjectForInstructionScripts(input: Object): string {
}

const precedencePlaceholderStart = stringToPrecomputedChunk(
'<style data-rprec="',
'<style data-precedence="',
);
const precedencePlaceholderEnd = stringToPrecomputedChunk('"></style>');

Expand All @@ -2268,6 +2268,13 @@ export function writeInitialResources(
resources: Resources,
responseState: ResponseState,
): boolean {
function flushLinkResource(resource) {
if (!resource.flushed) {
pushLinkImpl(target, resource.props, responseState);
resource.flushed = true;
}
}

const target = [];

const {
Expand Down Expand Up @@ -2307,13 +2314,7 @@ export function writeInitialResources(
}
});

usedStylePreloads.forEach(r => {
// style preloads could very likely have been flushed already so we check
if (!r.flushed) {
pushLinkImpl(target, r.props, responseState);
r.flushed = true;
}
});
usedStylePreloads.forEach(flushLinkResource);
usedStylePreloads.clear();

scripts.forEach(r => {
Expand All @@ -2325,29 +2326,13 @@ export function writeInitialResources(
});
scripts.clear();

usedScriptPreloads.forEach(r => {
// may have been flushed so we check
if (!r.flushed) {
pushLinkImpl(target, r.props, responseState);
r.flushed = true;
}
});
usedScriptPreloads.forEach(flushLinkResource);
usedScriptPreloads.clear();

explicitStylePreloads.forEach(r => {
if (!r.flushed) {
pushLinkImpl(target, r.props, responseState);
r.flushed = true;
}
});
explicitStylePreloads.forEach(flushLinkResource);
explicitStylePreloads.clear();

explicitScriptPreloads.forEach(r => {
if (!r.flushed) {
pushLinkImpl(target, r.props, responseState);
r.flushed = true;
}
});
explicitScriptPreloads.forEach(flushLinkResource);
explicitScriptPreloads.clear();

let i;
Expand All @@ -2366,6 +2351,13 @@ export function writeImmediateResources(
resources: Resources,
responseState: ResponseState,
): boolean {
function flushLinkResource(resource) {
if (!resource.flushed) {
pushLinkImpl(target, resource.props, responseState);
resource.flushed = true;
}
}

const target = [];

const {
Expand All @@ -2384,13 +2376,7 @@ export function writeImmediateResources(
});
fontPreloads.clear();

usedStylePreloads.forEach(r => {
// style preloads could very likely have been flushed already so we check
if (!r.flushed) {
pushLinkImpl(target, r.props, responseState);
r.flushed = true;
}
});
usedStylePreloads.forEach(flushLinkResource);
usedStylePreloads.clear();

scripts.forEach(r => {
Expand All @@ -2402,29 +2388,13 @@ export function writeImmediateResources(
});
scripts.clear();

usedScriptPreloads.forEach(r => {
// may have been flushed so we check
if (!r.flushed) {
pushLinkImpl(target, r.props, responseState);
r.flushed = true;
}
});
usedScriptPreloads.forEach(flushLinkResource);
usedScriptPreloads.clear();

explicitStylePreloads.forEach(r => {
if (!r.flushed) {
pushLinkImpl(target, r.props, responseState);
r.flushed = true;
}
});
explicitStylePreloads.forEach(flushLinkResource);
explicitStylePreloads.clear();

explicitScriptPreloads.forEach(r => {
if (!r.flushed) {
pushLinkImpl(target, r.props, responseState);
r.flushed = true;
}
});
explicitScriptPreloads.forEach(flushLinkResource);
explicitScriptPreloads.clear();

let i;
Expand Down Expand Up @@ -2548,7 +2518,7 @@ function writeStyleResourceDependency(
case 'href':
case 'rel':
case 'precedence':
case 'data-rprec': {
case 'data-precedence': {
break;
}
case 'children':
Expand Down
Loading

0 comments on commit 843f7c4

Please sign in to comment.