From 99efc627a5a8cb56f50cfffee544c86c49572b6f Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 23 May 2025 10:09:41 -0400 Subject: [PATCH 1/3] [eslint] Add an option to require dependencies on effect hooks (#33344) Summary: To prepare for automatic effect dependencies, some codebases may want to codemod existing useEffect calls with no deps to include an explicit undefined second argument in order to preserve the "run on every render" behavior. In sufficiently large codebases, this may require a temporary enforcement period where all effects provide an explicit dependencies argument. Outside of migration, relying on a component to render can lead to real bugs, especially when working with memoization. --- .../__tests__/ESLintRuleExhaustiveDeps-test.js | 17 +++++++++++++++++ .../src/rules/ExhaustiveDeps.ts | 15 +++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 8c5eeb004593a..555c54148ec1e 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -8344,6 +8344,23 @@ const testsTypescript = { }, ], }, + { + code: normalizeIndent` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + }); + } + `, + options: [{requireExplicitEffectDeps: true}], + errors: [ + { + message: + 'React Hook useEffect always requires dependencies. Please add a dependency array or an explicit `undefined`', + suggestions: undefined, + }, + ], + }, ], }; diff --git a/packages/eslint-plugin-react-hooks/src/rules/ExhaustiveDeps.ts b/packages/eslint-plugin-react-hooks/src/rules/ExhaustiveDeps.ts index 624d28e3b332c..d59a1ff79202c 100644 --- a/packages/eslint-plugin-react-hooks/src/rules/ExhaustiveDeps.ts +++ b/packages/eslint-plugin-react-hooks/src/rules/ExhaustiveDeps.ts @@ -67,6 +67,9 @@ const rule = { type: 'string', }, }, + requireExplicitEffectDeps: { + type: 'boolean', + } }, }, ], @@ -90,10 +93,13 @@ const rule = { ? rawOptions.experimental_autoDependenciesHooks : []; + const requireExplicitEffectDeps: boolean = rawOptions && rawOptions.requireExplicitEffectDeps || false; + const options = { additionalHooks, experimental_autoDependenciesHooks, enableDangerousAutofixThisMayCauseInfiniteLoops, + requireExplicitEffectDeps, }; function reportProblem(problem: Rule.ReportDescriptor) { @@ -1340,6 +1346,15 @@ const rule = { return; } + if (!maybeNode && isEffect && options.requireExplicitEffectDeps) { + reportProblem({ + node: reactiveHook, + message: + `React Hook ${reactiveHookName} always requires dependencies. ` + + `Please add a dependency array or an explicit \`undefined\`` + }); + } + const isAutoDepsHook = options.experimental_autoDependenciesHooks.includes(reactiveHookName); From 6a1dfe37776e5a41f4c1e07c33cf1f26c4a82979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 23 May 2025 13:25:13 -0400 Subject: [PATCH 2/3] Disable moveBefore experiment (#33348) There seems to be some bugs still to work out in Chrome. See #33187. Additionally, since you can't really rely on this function existing across browsers, it's hard to depend on its behavior anyway. In fact, you now have a source of inconsistent behaviors across browsers to deal with. Ideally it would also be more widely spread in fake DOM implementations like JSDOM so that we can use it unconditionally. #33177. We still want to enable this since it's a great feature but maybe not until it's more widely available cross-browsers with fewer bugs. --- packages/shared/ReactFeatureFlags.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 216b6d668a95f..d8b5f448d5297 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -194,7 +194,7 @@ export const disableLegacyContext = true; export const disableLegacyContextForFunctionComponents = true; // Enable the moveBefore() alternative to insertBefore(). This preserves states of moves. -export const enableMoveBefore = __EXPERIMENTAL__; +export const enableMoveBefore = false; // Disabled caching behavior of `react/cache` in client runtimes. export const disableClientCache = true; From c0464aedb16b1c970d717651bba8d1c66c578729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 23 May 2025 13:26:02 -0400 Subject: [PATCH 3/3] [Fizz] Block on Suspensey Fonts during reveal (#33342) This is the same technique we do for the client except we don't check whether this is newly created font loading to keep code small. Unfortunately, we can't use this technique for Suspensey images. They'll need to block before we call `startViewTransition` in a separate refactor. This is due to a bug in Chrome where `img.decode()` doesn't resolve until `startViewTransition` does. --- ...ReactDOMFizzInstructionSetInlineCodeStrings.js | 2 +- .../ReactDOMFizzInstructionSetShared.js | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js index 363e11f988d5b..e74e4bbe86b28 100644 --- a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js +++ b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js @@ -8,7 +8,7 @@ export const clientRenderBoundary = export const completeBoundary = '$RB=[];$RV=function(c){$RT=performance.now();for(var a=0;a { + revealBoundaries( + batch, + // Force layout to trigger font loading, we pass the actual value to trick minifiers. + document.documentElement.clientHeight, + ); + return Promise.race([ + // Block on fonts finishing loading before revealing these boundaries. + document.fonts.ready, + new Promise(resolve => setTimeout(resolve, SUSPENSEY_FONT_TIMEOUT)), + ]); + }, types: [], // TODO: Add a hard coded type for Suspense reveals. })); transition.ready.finally(() => {