Skip to content

Commit 8d5047e

Browse files
committed
Fix: Insert stylesheets that previously suspended
Follow up to #26450. In `acquireResource`, if a stylesheet resource already has an instance, we need to confirm that it was inserted into the document, because it may have been suspended. This happens because stylesheet resources are assigned a resource before committing, inside `suspendResource`. Probably a factoring a smell. As currently implemented, the idea is that `suspendResource` does all the same stuff as `acquireResource` except for the insertion.
1 parent d12bdcd commit 8d5047e

File tree

2 files changed

+32
-2
lines changed

2 files changed

+32
-2
lines changed

packages/react-dom-bindings/src/client/ReactDOMHostConfig.js

+31-2
Original file line numberDiff line numberDiff line change
@@ -1788,7 +1788,7 @@ type StyleTagResource = TResource<'style', null>;
17881788
type StyleResource = StyleTagResource | StylesheetResource;
17891789
type ScriptResource = TResource<'script', null>;
17901790
type VoidResource = TResource<'void', null>;
1791-
type Resource = StyleResource | ScriptResource | VoidResource;
1791+
export type Resource = StyleResource | ScriptResource | VoidResource;
17921792

17931793
type LoadingState = number;
17941794
const NotLoaded = /* */ 0b000;
@@ -2170,6 +2170,7 @@ function preinit(href: string, options: PreinitOptions) {
21702170
state.loading |= Errored;
21712171
});
21722172

2173+
state.loading |= Inserted;
21732174
insertStylesheet(instance, precedence, resourceRoot);
21742175
}
21752176

@@ -2518,6 +2519,11 @@ export function acquireResource(
25182519

25192520
markNodeAsHoistable(instance);
25202521
setInitialProperties(instance, 'style', styleProps);
2522+
2523+
// TODO: `style` does not have loading state for tracking insertions. I
2524+
// guess because these aren't suspensey? Not sure whether this is a
2525+
// factoring smell.
2526+
// resource.state.loading |= Inserted;
25212527
insertStylesheet(instance, qualifiedProps.precedence, hoistableRoot);
25222528
resource.instance = instance;
25232529

@@ -2556,6 +2562,7 @@ export function acquireResource(
25562562
linkInstance.onerror = reject;
25572563
});
25582564
setInitialProperties(instance, 'link', stylesheetProps);
2565+
resource.state.loading |= Inserted;
25592566
insertStylesheet(instance, qualifiedProps.precedence, hoistableRoot);
25602567
resource.instance = instance;
25612568

@@ -2604,6 +2611,28 @@ export function acquireResource(
26042611
);
26052612
}
26062613
}
2614+
} else {
2615+
// In the case of stylesheets, they might have already been assigned an
2616+
// instance during `suspendResource`. But that doesn't mean they were
2617+
// inserted, because the commit might have been interrupted. So we need to
2618+
// check now.
2619+
//
2620+
// The other resource types are unaffected because they are not
2621+
// yet suspensey.
2622+
//
2623+
// TODO: This is a bit of a code smell. Consider refactoring how
2624+
// `suspendResource` and `acquireResource` work together. The idea is that
2625+
// `suspendResource` does all the same stuff as `acquireResource` except
2626+
// for the insertion.
2627+
if (
2628+
resource.type === 'stylesheet' &&
2629+
(resource.state.loading & Inserted) === NotLoaded
2630+
) {
2631+
const qualifiedProps: StylesheetQualifyingProps = props;
2632+
const instance: Instance = resource.instance;
2633+
resource.state.loading |= Inserted;
2634+
insertStylesheet(instance, qualifiedProps.precedence, hoistableRoot);
2635+
}
26072636
}
26082637
return resource.instance;
26092638
}
@@ -2613,7 +2642,7 @@ export function releaseResource(resource: Resource): void {
26132642
}
26142643
26152644
function insertStylesheet(
2616-
instance: HTMLElement,
2645+
instance: Element,
26172646
precedence: string,
26182647
root: HoistableRoot,
26192648
): void {

packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js

+1
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ export const errorHydratingContainer = $$$hostConfig.errorHydratingContainer;
208208
// (optional)
209209
// -------------------
210210
export type HoistableRoot = mixed;
211+
export type Resource = mixed; // eslint-disable-line no-undef
211212
export const supportsResources = $$$hostConfig.supportsResources;
212213
export const isHostHoistableType = $$$hostConfig.isHostHoistableType;
213214
export const getHoistableRoot = $$$hostConfig.getHoistableRoot;

0 commit comments

Comments
 (0)