@samitouri / QOS-React / commits / fdad813ac7

[Float][Fiber] Enable Float methods to be called outside of render (#26557)

Stacked on #26570 Previously we restricted Float methods to only being callable while rendering. This allowed us to make associations between calls and their position in the DOM tree, for instance hoisting preinitialized styles into a ShadowRoot or an iframe Document. When considering how we are going to support Flight support in Float however it became clear that this restriction would lead to compromises on the implementation because the Flight client does not execute within the context of a client render. We want to be able to disaptch Float directives coming from Flight as soon as possible and this requires being able to call them outside of render. this patch modifies Float so that its methods are callable anywhere. The main consequence of this change is Float will always use the Document the renderer script is running within as the HoistableRoot. This means if you preinit as style inside a component render targeting a ShadowRoot the style will load in the ownerDocument not the ShadowRoot. Practially speaking it means that preinit is not useful inside ShadowRoots and iframes. This tradeoff was deemed acceptable because these methods are optimistic, not critical. Additionally, the other methods, preconntect, prefetchDNS, and preload, are not impacted because they already operated at the level of the ownerDocument and really only interface with the Network cache layer. I added a couple additional fixes that were necessary for getting tests to pass that are worth considering separately. The first commit improves the diff for `waitForThrow` so it compares strings if possible. The second commit makes invokeGuardedCallback not use metaprogramming pattern and swallows any novel errors produced from trying to run the guarded callback. Swallowing may not be the best we can do but it at least protects React against rapid failure when something causes the dispatchEvent to throw.

Josh Story committed Apr 20, 2023 at 14:40 UTC fdad813ac765e901e2957b8d36fba87e5504e5f4
11 files changed +77 -164
packages/react-art/src/ReactFiberConfigART.js
-8
@@ -479,11 +479,3 @@ export function suspendInstance(type, props) {}
479 export function waitForCommitToBeReady() {
480 return null;
481 }
482 -// eslint-disable-next-line no-undef
483 -export function prepareRendererToRender(container: Container): void {
484 - // noop
485 -}
486 -
487 -export function resetRendererAfterRender(): void {
488 - // noop
489 -}
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+28 -110
@@ -25,8 +25,6 @@ import {ConcurrentMode, NoMode} from 'react-reconciler/src/ReactTypeOfMode';
25
26 import hasOwnProperty from 'shared/hasOwnProperty';
27 import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
28 -import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals.js';
29 -const {Dispatcher} = ReactDOMSharedInternals;
28
29 import {
30 precacheFiberNode,
@@ -1936,31 +1934,6 @@ export function prepareToCommitHoistables() {
1934 tagCaches = null;
1935 }
1936
1939 -// It is valid to preload even when we aren't actively rendering. For cases where Float functions are
1940 -// called when there is no rendering we track the last used document. It is not safe to insert
1941 -// arbitrary resources into the lastCurrentDocument b/c it may not actually be the document
1942 -// that the resource is meant to apply too (for example stylesheets or scripts). This is only
1943 -// appropriate for resources that don't really have a strict tie to the document itself for example
1944 -// preloads
1945 -let lastCurrentDocument: ?Document = null;
1946 -let previousDispatcher = null;
1947 -export function prepareRendererToRender(rootContainer: Container) {
1948 - if (enableFloat) {
1949 - const rootNode = getHoistableRoot(rootContainer);
1950 - lastCurrentDocument = getDocumentFromRoot(rootNode);
1951 -
1952 - previousDispatcher = Dispatcher.current;
1953 - Dispatcher.current = ReactDOMClientDispatcher;
1954 - }
1955 -}
1956 -
1957 -export function resetRendererAfterRender() {
1958 - if (enableFloat) {
1959 - Dispatcher.current = previousDispatcher;
1960 - previousDispatcher = null;
1961 - }
1962 -}
1963 -
1937 // global collections of Resources
1938 const preloadPropsMap: Map<string, PreloadProps> = new Map();
1939 const preconnectsSet: Set<string> = new Set();
@@ -1982,25 +1955,6 @@ function getCurrentResourceRoot(): null | HoistableRoot {
1955 return currentContainer ? getHoistableRoot(currentContainer) : null;
1956 }
1957
1985 -// Preloads are somewhat special. Even if we don't have the Document
1986 -// used by the root that is rendering a component trying to insert a preload
1987 -// we can still seed the file cache by doing the preload on any document we have
1988 -// access to. We prefer the currentDocument if it exists, we also prefer the
1989 -// lastCurrentDocument if that exists. As a fallback we will use the window.document
1990 -// if available.
1991 -function getDocumentForPreloads(): ?Document {
1992 - const root = getCurrentResourceRoot();
1993 - if (root) {
1994 - return root.ownerDocument || root;
1995 - } else {
1996 - try {
1997 - return lastCurrentDocument || window.document;
1998 - } catch (error) {
1999 - return null;
2000 - }
2001 - }
2002 -}
2003 -
1958 function getDocumentFromRoot(root: HoistableRoot): Document {
1959 return root.ownerDocument || root;
1960 }
@@ -2015,13 +1969,23 @@ export const ReactDOMClientDispatcher = {
1969 preinit,
1970 };
1971
1972 +// We expect this to get inlined. It is a function mostly to communicate the special nature of
1973 +// how we resolve the HoistableRoot for ReactDOM.pre*() methods. Because we support calling
1974 +// these methods outside of render there is no way to know which Document or ShadowRoot is 'scoped'
1975 +// and so we have to fall back to something universal. Currently we just refer to the global document.
1976 +// This is notable because nowhere else in ReactDOM do we actually reference the global document or window
1977 +// because we may be rendering inside an iframe.
1978 +function getDocumentForImperativeFloatMethods(): Document {
1979 + return document;
1980 +}
1981 +
1982 function preconnectAs(
1983 rel: 'preconnect' | 'dns-prefetch',
1984 crossOrigin: null | '' | 'use-credentials',
1985 href: string,
1986 ) {
2023 - const ownerDocument = getDocumentForPreloads();
2024 - if (typeof href === 'string' && href && ownerDocument) {
1987 + const ownerDocument = getDocumentForImperativeFloatMethods();
1988 + if (typeof href === 'string' && href) {
1989 const limitedEscapedHref =
1990 escapeSelectorAttributeValueInsideDoubleQuotes(href);
1991 let key = `link[rel="${rel}"][href="${limitedEscapedHref}"]`;
@@ -2043,6 +2007,9 @@ function preconnectAs(
2007 }
2008
2009 function prefetchDNS(href: string, options?: mixed) {
2010 + if (!enableFloat) {
2011 + return;
2012 + }
2013 if (__DEV__) {
2014 if (typeof href !== 'string' || !href) {
2015 console.error(
@@ -2105,10 +2072,13 @@ type PreloadOptions = {
2072 type?: string,
2073 };
2074 function preload(href: string, options: PreloadOptions) {
2075 + if (!enableFloat) {
2076 + return;
2077 + }
2078 if (__DEV__) {
2079 validatePreloadArguments(href, options);
2080 }
2111 - const ownerDocument = getDocumentForPreloads();
2081 + const ownerDocument = getDocumentForImperativeFloatMethods();
2082 if (
2083 typeof href === 'string' &&
2084 href &&
@@ -2166,9 +2136,13 @@ type PreinitOptions = {
2136 integrity?: string,
2137 };
2138 function preinit(href: string, options: PreinitOptions) {
2139 + if (!enableFloat) {
2140 + return;
2141 + }
2142 if (__DEV__) {
2143 validatePreinitArguments(href, options);
2144 }
2145 + const ownerDocument = getDocumentForImperativeFloatMethods();
2146
2147 if (
2148 typeof href === 'string' &&
@@ -2176,51 +2150,11 @@ function preinit(href: string, options: PreinitOptions) {
2150 typeof options === 'object' &&
2151 options !== null
2152 ) {
2179 - const resourceRoot = getCurrentResourceRoot();
2153 const as = options.as;
2181 - if (!resourceRoot) {
2182 - if (as === 'style' || as === 'script') {
2183 - // We are going to emit a preload as a best effort fallback since this preinit
2184 - // was called outside of a render. Given the passive nature of this fallback
2185 - // we do not warn in dev when props disagree if there happens to already be a
2186 - // matching preload with this href
2187 - const preloadDocument = getDocumentForPreloads();
2188 - if (preloadDocument) {
2189 - const limitedEscapedHref =
2190 - escapeSelectorAttributeValueInsideDoubleQuotes(href);
2191 - const preloadKey = `link[rel="preload"][as="${as}"][href="${limitedEscapedHref}"]`;
2192 - let key = preloadKey;
2193 - switch (as) {
2194 - case 'style':
2195 - key = getStyleKey(href);
2196 - break;
2197 - case 'script':
2198 - key = getScriptKey(href);
2199 - break;
2200 - }
2201 - if (!preloadPropsMap.has(key)) {
2202 - const preloadProps = preloadPropsFromPreinitOptions(
2203 - href,
2204 - as,
2205 - options,
2206 - );
2207 - preloadPropsMap.set(key, preloadProps);
2208 -
2209 - if (null === preloadDocument.querySelector(preloadKey)) {
2210 - const instance = preloadDocument.createElement('link');
2211 - setInitialProperties(instance, 'link', preloadProps);
2212 - markNodeAsHoistable(instance);
2213 - (preloadDocument.head: any).appendChild(instance);
2214 - }
2215 - }
2216 - }
2217 - }
2218 - return;
2219 - }
2154
2155 switch (as) {
2156 case 'style': {
2223 - const styles = getResourcesFromRoot(resourceRoot).hoistableStyles;
2157 + const styles = getResourcesFromRoot(ownerDocument).hoistableStyles;
2158
2159 const key = getStyleKey(href);
2160 const precedence = options.precedence || 'default';
@@ -2239,7 +2173,7 @@ function preinit(href: string, options: PreinitOptions) {
2173 };
2174
2175 // Attempt to hydrate instance from DOM
2242 - let instance: null | Instance = resourceRoot.querySelector(
2176 + let instance: null | Instance = ownerDocument.querySelector(
2177 getStylesheetSelectorFromKey(key),
2178 );
2179 if (instance) {
@@ -2255,7 +2189,6 @@ function preinit(href: string, options: PreinitOptions) {
2189 if (preloadProps) {
2190 adoptPreloadPropsForStylesheet(stylesheetProps, preloadProps);
2191 }
2258 - const ownerDocument = getDocumentFromRoot(resourceRoot);
2192 const link = (instance = ownerDocument.createElement('link'));
2193 markNodeAsHoistable(link);
2194 setInitialProperties(link, 'link', stylesheetProps);
@@ -2272,7 +2205,7 @@ function preinit(href: string, options: PreinitOptions) {
2205 });
2206
2207 state.loading |= Inserted;
2275 - insertStylesheet(instance, precedence, resourceRoot);
2208 + insertStylesheet(instance, precedence, ownerDocument);
2209 }
2210
2211 // Construct a Resource and cache it
@@ -2287,7 +2220,7 @@ function preinit(href: string, options: PreinitOptions) {
2220 }
2221 case 'script': {
2222 const src = href;
2290 - const scripts = getResourcesFromRoot(resourceRoot).hoistableScripts;
2223 + const scripts = getResourcesFromRoot(ownerDocument).hoistableScripts;
2224
2225 const key = getScriptKey(src);
2226
@@ -2300,7 +2233,7 @@ function preinit(href: string, options: PreinitOptions) {
2233 }
2234
2235 // Attempt to hydrate instance from DOM
2303 - let instance: null | Instance = resourceRoot.querySelector(
2236 + let instance: null | Instance = ownerDocument.querySelector(
2237 getScriptSelectorFromKey(key),
2238 );
2239 if (!instance) {
@@ -2311,7 +2244,6 @@ function preinit(href: string, options: PreinitOptions) {
2244 if (preloadProps) {
2245 adoptPreloadPropsForScript(scriptProps, preloadProps);
2246 }
2314 - const ownerDocument = getDocumentFromRoot(resourceRoot);
2247 instance = ownerDocument.createElement('script');
2248 markNodeAsHoistable(instance);
2249 setInitialProperties(instance, 'link', scriptProps);
@@ -2332,20 +2264,6 @@ function preinit(href: string, options: PreinitOptions) {
2264 }
2265 }
2266
2335 -function preloadPropsFromPreinitOptions(
2336 - href: string,
2337 - as: ResourceType,
2338 - options: PreinitOptions,
2339 -): PreloadProps {
2340 - return {
2341 - href,
2342 - rel: 'preload',
2343 - as,
2344 - crossOrigin: as === 'font' ? '' : options.crossOrigin,
2345 - integrity: options.integrity,
2346 - };
2347 -}
2348 -
2267 function stylesheetPropsFromPreinitOptions(
2268 href: string,
2269 precedence: string,
packages/react-dom/src/__tests__/ReactDOMFloat-test.js
+47 -3
@@ -3833,7 +3833,7 @@ body {
3833 });
3834
3835 // @gate enableFloat
3836 - it('creates a preload resource when ReactDOM.preinit(..., {as: "style" }) is called outside of render on the client', async () => {
3836 + it('creates a stylesheet resource in the ownerDocument when ReactDOM.preinit(..., {as: "style" }) is called outside of render on the client', async () => {
3837 function App() {
3838 React.useEffect(() => {
3839 ReactDOM.preinit('foo', {as: 'style'});
@@ -3851,11 +3851,55 @@ body {
3851 expect(getMeaningfulChildren(document)).toEqual(
3852 <html>
3853 <head>
3854 - <link rel="preload" href="foo" as="style" />
3854 + <link rel="stylesheet" href="foo" data-precedence="default" />
3855 + </head>
3856 + <body>foo</body>
3857 + </html>,
3858 + );
3859 + });
3860 +
3861 + // @gate enableFloat
3862 + it('creates a stylesheet resource in the ownerDocument when ReactDOM.preinit(..., {as: "style" }) is called outside of render on the client', async () => {
3863 + // This is testing behavior, but it shows that it is not a good idea to preinit inside a shadowRoot. The point is we are asserting a behavior
3864 + // you would want to avoid in a real app.
3865 + const shadow = document.body.attachShadow({mode: 'open'});
3866 + function ShadowComponent() {
3867 + ReactDOM.preinit('bar', {as: 'style'});
3868 + return null;
3869 + }
3870 + function App() {
3871 + React.useEffect(() => {
3872 + ReactDOM.preinit('foo', {as: 'style'});
3873 + }, []);
3874 + return (
3875 + <html>
3876 + <body>
3877 + foo
3878 + {ReactDOM.createPortal(
3879 + <div>
3880 + <ShadowComponent />
3881 + shadow
3882 + </div>,
3883 + shadow,
3884 + )}
3885 + </body>
3886 + </html>
3887 + );
3888 + }
3889 +
3890 + const root = ReactDOMClient.createRoot(document);
3891 + root.render(<App />);
3892 + await waitForAll([]);
3893 + expect(getMeaningfulChildren(document)).toEqual(
3894 + <html>
3895 + <head>
3896 + <link rel="stylesheet" href="bar" data-precedence="default" />
3897 + <link rel="stylesheet" href="foo" data-precedence="default" />
3898 </head>
3899 <body>foo</body>
3900 </html>,
3901 );
3902 + expect(getMeaningfulChildren(shadow)).toEqual(<div>shadow</div>);
3903 });
3904
3905 // @gate enableFloat
@@ -3955,7 +3999,7 @@ body {
3999 expect(getMeaningfulChildren(document)).toEqual(
4000 <html>
4001 <head>
3958 - <link rel="preload" href="foo" as="script" />
4002 + <script async="" src="foo" />
4003 </head>
4004 <body>foo</body>
4005 </html>,
packages/react-dom/src/client/ReactDOMRoot.js
+2 -8
@@ -247,11 +247,8 @@ export function createRoot(
247 transitionCallbacks,
248 );
249 markContainerAsRoot(root.current, container);
250 + Dispatcher.current = ReactDOMClientDispatcher;
251
251 - if (enableFloat) {
252 - // Set the default dispatcher to the client dispatcher
253 - Dispatcher.current = ReactDOMClientDispatcher;
254 - }
252 const rootContainerElement: Document | Element | DocumentFragment =
253 container.nodeType === COMMENT_NODE
254 ? (container.parentNode: any)
@@ -339,10 +336,7 @@ export function hydrateRoot(
336 transitionCallbacks,
337 );
338 markContainerAsRoot(root.current, container);
342 - if (enableFloat) {
343 - // Set the default dispatcher to the client dispatcher
344 - Dispatcher.current = ReactDOMClientDispatcher;
345 - }
339 + Dispatcher.current = ReactDOMClientDispatcher;
340 // This can't be a comment node since hydration doesn't work on comment nodes anyway.
341 listenToAllSupportedEvents(container);
342
packages/react-native-renderer/src/ReactFiberConfigFabric.js
-8
@@ -489,11 +489,3 @@ export function suspendInstance(type: Type, props: Props): void {}
489 export function waitForCommitToBeReady(): null {
490 return null;
491 }
492 -
493 -export function prepareRendererToRender(container: Container): void {
494 - // noop
495 -}
496 -
497 -export function resetRendererAfterRender() {
498 - // noop
499 -}
packages/react-native-renderer/src/ReactFiberConfigNative.js
-8
@@ -542,11 +542,3 @@ export function suspendInstance(type: Type, props: Props): void {}
542 export function waitForCommitToBeReady(): null {
543 return null;
544 }
545 -
546 -export function prepareRendererToRender(container: Container): void {
547 - // noop
548 -}
549 -
550 -export function resetRendererAfterRender(): void {
551 - // noop
552 -}
packages/react-noop-renderer/src/createReactNoop.js
-3
@@ -629,9 +629,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
629 },
630
631 waitForCommitToBeReady,
632 -
633 - prepareRendererToRender() {},
634 - resetRendererAfterRender() {},
632 };
633
634 const hostConfig = useMutation
packages/react-reconciler/src/ReactFiberWorkLoop.js
-4
@@ -72,8 +72,6 @@ import {
72 afterActiveInstanceBlur,
73 getCurrentEventPriority,
74 errorHydratingContainer,
75 - prepareRendererToRender,
76 - resetRendererAfterRender,
75 startSuspendingCommit,
76 waitForCommitToBeReady,
77 preloadInstance,
@@ -1757,7 +1755,6 @@ export function shouldRemainOnPreviousScreen(): boolean {
1755 }
1756
1757 function pushDispatcher(container: any) {
1760 - prepareRendererToRender(container);
1758 const prevDispatcher = ReactCurrentDispatcher.current;
1759 ReactCurrentDispatcher.current = ContextOnlyDispatcher;
1760 if (prevDispatcher === null) {
@@ -1771,7 +1768,6 @@ function pushDispatcher(container: any) {
1768 }
1769
1770 function popDispatcher(prevDispatcher: any) {
1774 - resetRendererAfterRender();
1771 ReactCurrentDispatcher.current = prevDispatcher;
1772 }
1773
packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js
-2
@@ -85,8 +85,6 @@ describe('ReactFiberHostContext', () => {
85 waitForCommitToBeReady() {
86 return null;
87 },
88 - prepareRendererToRender: function () {},
89 - resetRendererAfterRender: function () {},
88 supportsMutation: true,
89 });
90
packages/react-reconciler/src/forks/ReactFiberConfig.custom.js
-2
@@ -75,8 +75,6 @@ export const preloadInstance = $$$config.preloadInstance;
75 export const startSuspendingCommit = $$$config.startSuspendingCommit;
76 export const suspendInstance = $$$config.suspendInstance;
77 export const waitForCommitToBeReady = $$$config.waitForCommitToBeReady;
78 -export const prepareRendererToRender = $$$config.prepareRendererToRender;
79 -export const resetRendererAfterRender = $$$config.resetRendererAfterRender;
78
79 // -------------------
80 // Microtasks
packages/react-test-renderer/src/ReactFiberConfigTestHost.js
-8
@@ -343,11 +343,3 @@ export function suspendInstance(type: Type, props: Props): void {}
343 export function waitForCommitToBeReady(): null {
344 return null;
345 }
346 -
347 -export function prepareRendererToRender(container: Container): void {
348 - // noop
349 -}
350 -
351 -export function resetRendererAfterRender(): void {
352 - // noop
353 -}