From bfaeb4a46175fa0f4edf2eba58349d5029e5e86e Mon Sep 17 00:00:00 2001 From: 0xFango Date: Fri, 23 May 2025 09:02:39 +0700 Subject: [PATCH 1/3] Fix incorrect use of NoLanes in executionContext check (#33170) ## Summary This PR fixes a likely incorrect condition in the `scheduleUpdateOnFiber` function inside `ReactFiberWorkLoop.js`. Previously, the code checked: ```js (executionContext & RenderContext) !== NoLanes ```` However, `NoLanes` is part of the lane priority system, not the execution context flags. The intent here seems to be to detect whether the current execution context includes `RenderContext`, which should be compared against `NoContext`, not `NoLanes`. This fix replaces `NoLanes` with `NoContext` for semantic correctness and consistency with other checks throughout the codebase. **Fixes [[#33169](https://github.com/facebook/react/issues/33169)](https://github.com/facebook/react/issues/33169)** --- ## How did you test this change? I ran the following commands to validate correctness and ensure nothing was broken: * `yarn lint` * `yarn linc` * `yarn test` * `yarn test --prod` * `yarn flow` * `yarn prettier` All checks passed. Since this is a minor internal logic fix and doesn't change public behavior or APIs, no additional tests are necessary at this time. --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index ecf22aaad0..fd74620975 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -908,7 +908,7 @@ export function scheduleUpdateOnFiber( markRootUpdated(root, lane); if ( - (executionContext & RenderContext) !== NoLanes && + (executionContext & RenderContext) !== NoContext && root === workInProgressRoot ) { // This update was dispatched during the render phase. This is a mistake From 99efc627a5a8cb56f50cfffee544c86c49572b6f Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 23 May 2025 10:09:41 -0400 Subject: [PATCH 2/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 8c5eeb0045..555c54148e 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 624d28e3b3..d59a1ff792 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 51620ac79c37c5b105df6cc55786ec591d6a4dc5 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 23 May 2025 13:12:02 -0400 Subject: [PATCH 3/3] [compiler][gating] Custom opt out directives (experimental option) Adding an experimental / unstable compiler config to enable custom opt-out directives --- .../src/Entrypoint/Options.ts | 25 ++++++++++++ .../src/Entrypoint/Program.ts | 17 ++++++++- .../src/Utils/TestUtils.ts | 38 +++++++++---------- .../custom-opt-out-directive.expect.md | 35 +++++++++++++++++ .../compiler/custom-opt-out-directive.tsx | 10 +++++ 5 files changed, 104 insertions(+), 21 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/custom-opt-out-directive.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/custom-opt-out-directive.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts index 96cce887d8..0c23ceb345 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts @@ -41,6 +41,10 @@ const DynamicGatingOptionsSchema = z.object({ source: z.string(), }); export type DynamicGatingOptions = z.infer; +const CustomOptOutDirectiveSchema = z + .nullable(z.array(z.string())) + .default(null); +type CustomOptOutDirective = z.infer; export type PluginOptions = { environment: EnvironmentConfig; @@ -132,6 +136,11 @@ export type PluginOptions = { */ ignoreUseNoForget: boolean; + /** + * Unstable / do not use + */ + customOptOutDirectives: CustomOptOutDirective; + sources: Array | ((filename: string) => boolean) | null; /** @@ -278,6 +287,7 @@ export const defaultOptions: PluginOptions = { return filename.indexOf('node_modules') === -1; }, enableReanimatedCheck: true, + customOptOutDirectives: null, target: '19', } as const; @@ -338,6 +348,21 @@ export function parsePluginOptions(obj: unknown): PluginOptions { } break; } + case 'customOptOutDirectives': { + const result = CustomOptOutDirectiveSchema.safeParse(value); + if (result.success) { + parsedOptions[key] = result.data; + } else { + CompilerError.throwInvalidConfig({ + reason: + 'Could not parse custom opt out directives. Update React Compiler config to fix the error', + description: `${fromZodError(result.error)}`, + loc: null, + suggestions: null, + }); + } + break; + } default: { parsedOptions[key] = value; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index cb57bd2c49..de8d16fb12 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -63,7 +63,16 @@ export function tryFindDirectiveEnablingMemoization( export function findDirectiveDisablingMemoization( directives: Array, + {customOptOutDirectives}: PluginOptions, ): t.Directive | null { + if (customOptOutDirectives != null) { + return ( + directives.find( + directive => + customOptOutDirectives.indexOf(directive.value.value) !== -1, + ) ?? null + ); + } return ( directives.find(directive => OPT_OUT_DIRECTIVES.has(directive.value.value), @@ -394,7 +403,8 @@ export function compileProgram( code: pass.code, suppressions, hasModuleScopeOptOut: - findDirectiveDisablingMemoization(program.node.directives) != null, + findDirectiveDisablingMemoization(program.node.directives, pass.opts) != + null, }); const queue: Array = findFunctionsToCompile( @@ -571,7 +581,10 @@ function processFn( } directives = { optIn: optIn.unwrapOr(null), - optOut: findDirectiveDisablingMemoization(fn.node.body.directives), + optOut: findDirectiveDisablingMemoization( + fn.node.body.directives, + programContext.opts, + ), }; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts index b4484331de..6c2cfd5d07 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts @@ -93,6 +93,21 @@ const testComplexConfigDefaults: PartialEnvironmentConfig = { }, ], }; + +function* splitPragma( + pragma: string, +): Generator<{key: string; value: string | null}> { + for (const entry of pragma.split('@')) { + const keyVal = entry.trim(); + const valIdx = keyVal.indexOf(':'); + if (valIdx === -1) { + yield {key: keyVal.split(' ', 1)[0], value: null}; + } else { + yield {key: keyVal.slice(0, valIdx), value: keyVal.slice(valIdx + 1)}; + } + } +} + /** * For snap test fixtures and playground only. */ @@ -101,19 +116,11 @@ function parseConfigPragmaEnvironmentForTest( ): EnvironmentConfig { const maybeConfig: Partial> = {}; - for (const token of pragma.split(' ')) { - if (!token.startsWith('@')) { - continue; - } - const keyVal = token.slice(1); - const valIdx = keyVal.indexOf(':'); - const key = valIdx === -1 ? keyVal : keyVal.slice(0, valIdx); - const val = valIdx === -1 ? undefined : keyVal.slice(valIdx + 1); - const isSet = val === undefined || val === 'true'; + for (const {key, value: val} of splitPragma(pragma)) { if (!hasOwnProperty(EnvironmentConfigSchema.shape, key)) { continue; } - + const isSet = val == null || val === 'true'; if (isSet && key in testComplexConfigDefaults) { maybeConfig[key] = testComplexConfigDefaults[key]; } else if (isSet) { @@ -176,18 +183,11 @@ export function parseConfigPragmaForTests( compilationMode: defaults.compilationMode, environment, }; - for (const token of pragma.split(' ')) { - if (!token.startsWith('@')) { - continue; - } - const keyVal = token.slice(1); - const idx = keyVal.indexOf(':'); - const key = idx === -1 ? keyVal : keyVal.slice(0, idx); - const val = idx === -1 ? undefined : keyVal.slice(idx + 1); + for (const {key, value: val} of splitPragma(pragma)) { if (!hasOwnProperty(defaultOptions, key)) { continue; } - const isSet = val === undefined || val === 'true'; + const isSet = val == null || val === 'true'; if (isSet && key in testComplexPluginOptionDefaults) { options[key] = testComplexPluginOptionDefaults[key]; } else if (isSet) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/custom-opt-out-directive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/custom-opt-out-directive.expect.md new file mode 100644 index 0000000000..7875137a88 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/custom-opt-out-directive.expect.md @@ -0,0 +1,35 @@ + +## Input + +```javascript +// @customOptOutDirectives:["use todo memo"] +function Component() { + 'use todo memo'; + return
hello world!
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +}; + +``` + +## Code + +```javascript +// @customOptOutDirectives:["use todo memo"] +function Component() { + "use todo memo"; + return
hello world!
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +}; + +``` + +### Eval output +(kind: ok)
hello world!
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/custom-opt-out-directive.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/custom-opt-out-directive.tsx new file mode 100644 index 0000000000..2255596183 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/custom-opt-out-directive.tsx @@ -0,0 +1,10 @@ +// @customOptOutDirectives:["use todo memo"] +function Component() { + 'use todo memo'; + return
hello world!
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +};