diff --git a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts index aa49bda22b27e..ff9817380fada 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts @@ -11,6 +11,7 @@ import { injectReanimatedFlag, pipelineUsesReanimatedPlugin, } from '../Entrypoint/Reanimated'; +import validateNoUntransformedReferences from '../Entrypoint/ValidateNoUntransformedReferences'; const ENABLE_REACT_COMPILER_TIMINGS = process.env['ENABLE_REACT_COMPILER_TIMINGS'] === '1'; @@ -61,12 +62,19 @@ export default function BabelPluginReactCompiler( }, }; } - compileProgram(prog, { + const result = compileProgram(prog, { opts, filename: pass.filename ?? null, comments: pass.file.ast.comments ?? [], code: pass.file.code, }); + validateNoUntransformedReferences( + prog, + pass.filename ?? null, + opts.logger, + opts.environment, + result?.retryErrors ?? [], + ); if (ENABLE_REACT_COMPILER_TIMINGS === true) { performance.mark(`${filename}:end`, { detail: 'BabelPlugin:Program:end', 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 0865b50f84116..72fa101260df5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -271,6 +271,9 @@ function isFilePartOfSources( return false; } +type CompileProgramResult = { + retryErrors: Array<{fn: BabelFn; error: CompilerError}>; +}; /** * `compileProgram` is directly invoked by the react-compiler babel plugin, so * exceptions thrown by this function will fail the babel build. @@ -285,16 +288,16 @@ function isFilePartOfSources( export function compileProgram( program: NodePath, pass: CompilerPass, -): void { +): CompileProgramResult | null { if (shouldSkipCompilation(program, pass)) { - return; + return null; } const environment = pass.opts.environment; const restrictedImportsErr = validateRestrictedImports(program, environment); if (restrictedImportsErr) { handleError(restrictedImportsErr, pass, null); - return; + return null; } const useMemoCacheIdentifier = program.scope.generateUidIdentifier('c'); @@ -365,7 +368,7 @@ export function compileProgram( filename: pass.filename ?? null, }, ); - + const retryErrors: Array<{fn: BabelFn; error: CompilerError}> = []; const processFn = ( fn: BabelFn, fnType: ReactFunctionType, @@ -429,7 +432,9 @@ export function compileProgram( handleError(compileResult.error, pass, fn.node.loc ?? null); } // If non-memoization features are enabled, retry regardless of error kind - if (!environment.enableFire) { + if ( + !(environment.enableFire || environment.inferEffectDependencies != null) + ) { return null; } try { @@ -448,6 +453,9 @@ export function compileProgram( }; } catch (err) { // TODO: we might want to log error here, but this will also result in duplicate logging + if (err instanceof CompilerError) { + retryErrors.push({fn, error: err}); + } return null; } } @@ -538,7 +546,7 @@ export function compileProgram( program.node.directives, ); if (moduleScopeOptOutDirectives.length > 0) { - return; + return null; } let gating: null | { gatingFn: ExternalFunction; @@ -596,7 +604,7 @@ export function compileProgram( } } catch (err) { handleError(err, pass, null); - return; + return null; } /* @@ -638,6 +646,7 @@ export function compileProgram( } addImportsToProgram(program, externalFunctions); } + return {retryErrors}; } function shouldSkipCompilation( diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts new file mode 100644 index 0000000000000..07ab3b2b6a172 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts @@ -0,0 +1,275 @@ +import {NodePath} from '@babel/core'; +import * as t from '@babel/types'; + +import { + CompilerError, + CompilerErrorDetailOptions, + EnvironmentConfig, + ErrorSeverity, + Logger, +} from '..'; +import {getOrInsertWith} from '../Utils/utils'; +import {Environment} from '../HIR'; +import {DEFAULT_EXPORT} from '../HIR/Environment'; + +function throwInvalidReact( + options: Omit, + {logger, filename}: TraversalState, +): never { + const detail: CompilerErrorDetailOptions = { + ...options, + severity: ErrorSeverity.InvalidReact, + }; + logger?.logEvent(filename, { + kind: 'CompileError', + fnLoc: null, + detail, + }); + CompilerError.throw(detail); +} +function assertValidEffectImportReference( + numArgs: number, + paths: Array>, + context: TraversalState, +): void { + for (const path of paths) { + const parent = path.parentPath; + if (parent != null && parent.isCallExpression()) { + const args = parent.get('arguments'); + /** + * Only error on untransformed references of the form `useMyEffect(...)` + * or `moduleNamespace.useMyEffect(...)`, with matching argument counts. + * TODO: do we also want a mode to also hard error on non-call references? + */ + if (args.length === numArgs) { + const maybeErrorDiagnostic = matchCompilerDiagnostic( + path, + context.transformErrors, + ); + /** + * Note that we cannot easily check the type of the first argument here, + * as it may have already been transformed by the compiler (and not + * memoized). + */ + throwInvalidReact( + { + reason: + '[InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. ' + + 'This will break your build! ' + + 'To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics.', + description: maybeErrorDiagnostic + ? `(Bailout reason: ${maybeErrorDiagnostic})` + : null, + loc: parent.node.loc ?? null, + }, + context, + ); + } + } + } +} + +function assertValidFireImportReference( + paths: Array>, + context: TraversalState, +): void { + if (paths.length > 0) { + const maybeErrorDiagnostic = matchCompilerDiagnostic( + paths[0], + context.transformErrors, + ); + throwInvalidReact( + { + reason: + '[Fire] Untransformed reference to compiler-required feature. ' + + 'Either remove this `fire` call or ensure it is successfully transformed by the compiler', + description: maybeErrorDiagnostic + ? `(Bailout reason: ${maybeErrorDiagnostic})` + : null, + loc: paths[0].node.loc ?? null, + }, + context, + ); + } +} +export default function validateNoUntransformedReferences( + path: NodePath, + filename: string | null, + logger: Logger | null, + env: EnvironmentConfig, + transformErrors: Array<{fn: NodePath; error: CompilerError}>, +): void { + const moduleLoadChecks = new Map< + string, + Map + >(); + if (env.enableFire) { + /** + * Error on any untransformed references to `fire` (e.g. including non-call + * expressions) + */ + for (const module of Environment.knownReactModules) { + const react = getOrInsertWith(moduleLoadChecks, module, () => new Map()); + react.set('fire', assertValidFireImportReference); + } + } + if (env.inferEffectDependencies) { + for (const { + function: {source, importSpecifierName}, + numRequiredArgs, + } of env.inferEffectDependencies) { + const module = getOrInsertWith(moduleLoadChecks, source, () => new Map()); + module.set( + importSpecifierName, + assertValidEffectImportReference.bind(null, numRequiredArgs), + ); + } + } + if (moduleLoadChecks.size > 0) { + transformProgram(path, moduleLoadChecks, filename, logger, transformErrors); + } +} + +type TraversalState = { + shouldInvalidateScopes: boolean; + program: NodePath; + logger: Logger | null; + filename: string | null; + transformErrors: Array<{fn: NodePath; error: CompilerError}>; +}; +type CheckInvalidReferenceFn = ( + paths: Array>, + context: TraversalState, +) => void; + +function validateImportSpecifier( + specifier: NodePath, + importSpecifierChecks: Map, + state: TraversalState, +): void { + const imported = specifier.get('imported'); + const specifierName: string = + imported.node.type === 'Identifier' + ? imported.node.name + : imported.node.value; + const checkFn = importSpecifierChecks.get(specifierName); + if (checkFn == null) { + return; + } + if (state.shouldInvalidateScopes) { + state.shouldInvalidateScopes = false; + state.program.scope.crawl(); + } + + const local = specifier.get('local'); + const binding = local.scope.getBinding(local.node.name); + CompilerError.invariant(binding != null, { + reason: 'Expected binding to be found for import specifier', + loc: local.node.loc ?? null, + }); + checkFn(binding.referencePaths, state); +} + +function validateNamespacedImport( + specifier: NodePath, + importSpecifierChecks: Map, + state: TraversalState, +): void { + if (state.shouldInvalidateScopes) { + state.shouldInvalidateScopes = false; + state.program.scope.crawl(); + } + const local = specifier.get('local'); + const binding = local.scope.getBinding(local.node.name); + const defaultCheckFn = importSpecifierChecks.get(DEFAULT_EXPORT); + + CompilerError.invariant(binding != null, { + reason: 'Expected binding to be found for import specifier', + loc: local.node.loc ?? null, + }); + const filteredReferences = new Map< + CheckInvalidReferenceFn, + Array> + >(); + for (const reference of binding.referencePaths) { + if (defaultCheckFn != null) { + getOrInsertWith(filteredReferences, defaultCheckFn, () => []).push( + reference, + ); + } + const parent = reference.parentPath; + if ( + parent != null && + parent.isMemberExpression() && + parent.get('object') === reference + ) { + if (parent.node.computed || parent.node.property.type !== 'Identifier') { + continue; + } + const checkFn = importSpecifierChecks.get(parent.node.property.name); + if (checkFn != null) { + getOrInsertWith(filteredReferences, checkFn, () => []).push(parent); + } + } + } + + for (const [checkFn, references] of filteredReferences) { + checkFn(references, state); + } +} +function transformProgram( + path: NodePath, + + moduleLoadChecks: Map>, + filename: string | null, + logger: Logger | null, + transformErrors: Array<{fn: NodePath; error: CompilerError}>, +): void { + const traversalState: TraversalState = { + shouldInvalidateScopes: true, + program: path, + filename, + logger, + transformErrors, + }; + path.traverse({ + ImportDeclaration(path: NodePath) { + const importSpecifierChecks = moduleLoadChecks.get( + path.node.source.value, + ); + if (importSpecifierChecks == null) { + return; + } + const specifiers = path.get('specifiers'); + for (const specifier of specifiers) { + if (specifier.isImportSpecifier()) { + validateImportSpecifier( + specifier, + importSpecifierChecks, + traversalState, + ); + } else { + validateNamespacedImport( + specifier as NodePath< + t.ImportNamespaceSpecifier | t.ImportDefaultSpecifier + >, + importSpecifierChecks, + traversalState, + ); + } + } + }, + }); +} + +function matchCompilerDiagnostic( + badReference: NodePath, + transformErrors: Array<{fn: NodePath; error: CompilerError}>, +): string | null { + for (const {fn, error} of transformErrors) { + if (fn.isAncestor(badReference)) { + return error.toString(); + } + } + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 4fce273b7a2de..1f64452e4bacf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -1121,6 +1121,7 @@ export class Environment { moduleName.toLowerCase() === 'react-dom' ); } + static knownReactModules: ReadonlyArray = ['react', 'react-dom']; getFallthroughPropertyType( receiver: Type, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn-default-import.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn-default-import.expect.md new file mode 100644 index 0000000000000..cd977ae834b8a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn-default-import.expect.md @@ -0,0 +1,26 @@ + +## Input + +```javascript +// @inferEffectDependencies @compilationMode(infer) @panicThreshold(none) +import useMyEffect from 'useEffectWrapper'; + +function nonReactFn(arg) { + useMyEffect(() => [1, 2, arg]); +} + +``` + + +## Error + +``` + 3 | + 4 | function nonReactFn(arg) { +> 5 | useMyEffect(() => [1, 2, arg]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (5:5) + 6 | } + 7 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn-default-import.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn-default-import.js new file mode 100644 index 0000000000000..8061b44a3dc2f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn-default-import.js @@ -0,0 +1,6 @@ +// @inferEffectDependencies @compilationMode(infer) @panicThreshold(none) +import useMyEffect from 'useEffectWrapper'; + +function nonReactFn(arg) { + useMyEffect(() => [1, 2, arg]); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.expect.md new file mode 100644 index 0000000000000..e36e9b3c0e681 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.expect.md @@ -0,0 +1,26 @@ + +## Input + +```javascript +// @inferEffectDependencies @compilationMode(infer) @panicThreshold(none) +import {useEffect} from 'react'; + +function nonReactFn(arg) { + useEffect(() => [1, 2, arg]); +} + +``` + + +## Error + +``` + 3 | + 4 | function nonReactFn(arg) { +> 5 | useEffect(() => [1, 2, arg]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (5:5) + 6 | } + 7 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.js new file mode 100644 index 0000000000000..813c3b6c3dccb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.js @@ -0,0 +1,6 @@ +// @inferEffectDependencies @compilationMode(infer) @panicThreshold(none) +import {useEffect} from 'react'; + +function nonReactFn(arg) { + useEffect(() => [1, 2, arg]); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.expect.md new file mode 100644 index 0000000000000..ee17fb048e7e7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +/** + * Error on non-inlined effect functions: + * 1. From the effect hook callee's perspective, it only makes sense + * to either + * (a) never hard error (i.e. failing to infer deps is acceptable) or + * (b) always hard error, + * regardless of whether the callback function is an inline fn. + * 2. (Technical detail) it's harder to support detecting cases in which + * function (pre-Forget transform) was inline but becomes memoized + */ +function Component({foo}) { + function f() { + console.log(foo); + } + + // No inferred dep array, the argument is not a lambda + useEffect(f); +} + +``` + + +## Error + +``` + 18 | + 19 | // No inferred dep array, the argument is not a lambda +> 20 | useEffect(f); + | ^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (20:20) + 21 | } + 22 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.js new file mode 100644 index 0000000000000..a011e3bf144e1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.js @@ -0,0 +1,21 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +/** + * Error on non-inlined effect functions: + * 1. From the effect hook callee's perspective, it only makes sense + * to either + * (a) never hard error (i.e. failing to infer deps is acceptable) or + * (b) always hard error, + * regardless of whether the callback function is an inline fn. + * 2. (Technical detail) it's harder to support detecting cases in which + * function (pre-Forget transform) was inline but becomes memoized + */ +function Component({foo}) { + function f() { + console.log(foo); + } + + // No inferred dep array, the argument is not a lambda + useEffect(f); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.expect.md new file mode 100644 index 0000000000000..71ecefeeea50f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import React from 'react'; + +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + React.useEffect(() => print(obj)); +} + +``` + + +## Error + +``` + 4 | function NonReactiveDepInEffect() { + 5 | const obj = makeObject_Primitives(); +> 6 | React.useEffect(() => print(obj)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (6:6) + 7 | } + 8 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.js similarity index 73% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.js index 0dbae754ecf76..237535fe5b4e2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.js @@ -1,4 +1,4 @@ -// @inferEffectDependencies +// @inferEffectDependencies @panicThreshold(none) import React from 'react'; function NonReactiveDepInEffect() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md new file mode 100644 index 0000000000000..575fd2101f77f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useSpecialEffect} from 'shared-runtime'; + +/** + * Note that a react compiler-based transform still has limitations on JS syntax. + * We should surface these as actionable lint / build errors to devs. + */ +function Component({prop1}) { + 'use memo'; + useSpecialEffect(() => { + try { + console.log(prop1); + } finally { + console.log('exiting'); + } + }, [prop1]); + return
{prop1}
; +} + +``` + + +## Error + +``` + 8 | function Component({prop1}) { + 9 | 'use memo'; +> 10 | useSpecialEffect(() => { + | ^^^^^^^^^^^^^^^^^^^^^^^^ +> 11 | try { + | ^^^^^^^^^ +> 12 | console.log(prop1); + | ^^^^^^^^^ +> 13 | } finally { + | ^^^^^^^^^ +> 14 | console.log('exiting'); + | ^^^^^^^^^ +> 15 | } + | ^^^^^^^^^ +> 16 | }, [prop1]); + | ^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics.. (Bailout reason: Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (11:15)) (10:16) + 17 | return
{prop1}
; + 18 | } + 19 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.js new file mode 100644 index 0000000000000..d3c83c25620f8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.js @@ -0,0 +1,18 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useSpecialEffect} from 'shared-runtime'; + +/** + * Note that a react compiler-based transform still has limitations on JS syntax. + * We should surface these as actionable lint / build errors to devs. + */ +function Component({prop1}) { + 'use memo'; + useSpecialEffect(() => { + try { + console.log(prop1); + } finally { + console.log('exiting'); + } + }, [prop1]); + return
{prop1}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.expect.md new file mode 100644 index 0000000000000..52b99a393e102 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +function Component({propVal}) { + 'use no memo'; + useEffect(() => [propVal]); +} + +``` + + +## Error + +``` + 4 | function Component({propVal}) { + 5 | 'use no memo'; +> 6 | useEffect(() => [propVal]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (6:6) + 7 | } + 8 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.js new file mode 100644 index 0000000000000..53eeda50501ed --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.js @@ -0,0 +1,7 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +function Component({propVal}) { + 'use no memo'; + useEffect(() => [propVal]); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md index 89da346a72c50..d0745f9bd37ba 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md @@ -34,13 +34,6 @@ function Component({foo, bar}) { console.log(bar.qux); }); - function f() { - console.log(foo); - } - - // No inferred dep array, the argument is not a lambda - useEffect(f); - useEffectWrapper(() => { console.log(foo); }); @@ -58,7 +51,7 @@ import useEffectWrapper from "useEffectWrapper"; const moduleNonReactive = 0; function Component(t0) { - const $ = _c(14); + const $ = _c(12); const { foo, bar } = t0; const ref = useRef(0); @@ -119,7 +112,7 @@ function Component(t0) { useEffect(t4, [bar.baz, bar.qux]); let t5; if ($[10] !== foo) { - t5 = function f() { + t5 = () => { console.log(foo); }; $[10] = foo; @@ -127,20 +120,7 @@ function Component(t0) { } else { t5 = $[11]; } - const f = t5; - - useEffect(f); - let t6; - if ($[12] !== foo) { - t6 = () => { - console.log(foo); - }; - $[12] = foo; - $[13] = t6; - } else { - t6 = $[13]; - } - useEffectWrapper(t6, [foo]); + useEffectWrapper(t5, [foo]); } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.js index efdd4164789b4..2bad5ee1cc920 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.js @@ -30,13 +30,6 @@ function Component({foo, bar}) { console.log(bar.qux); }); - function f() { - console.log(foo); - } - - // No inferred dep array, the argument is not a lambda - useEffect(f); - useEffectWrapper(() => { console.log(foo); }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md deleted file mode 100644 index a5a576c83067a..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md +++ /dev/null @@ -1,44 +0,0 @@ - -## Input - -```javascript -// @inferEffectDependencies -import React from 'react'; - -function NonReactiveDepInEffect() { - const obj = makeObject_Primitives(); - React.useEffect(() => print(obj)); -} - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies -import React from "react"; - -function NonReactiveDepInEffect() { - const $ = _c(2); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = makeObject_Primitives(); - $[0] = t0; - } else { - t0 = $[0]; - } - const obj = t0; - let t1; - if ($[1] === Symbol.for("react.memo_cache_sentinel")) { - t1 = () => print(obj); - $[1] = t1; - } else { - t1 = $[1]; - } - React.useEffect(t1); -} - -``` - -### Eval output -(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md new file mode 100644 index 0000000000000..0384d3335ca57 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useRef} from 'react'; +import {useSpecialEffect} from 'shared-runtime'; + +/** + * The retry pipeline disables memoization features, which means we need to + * provide an alternate implementation of effect dependencies which does not + * rely on memoization. + */ +function useFoo({cond}) { + const ref = useRef(); + const derived = cond ? ref.current : makeObject(); + useSpecialEffect(() => { + log(derived); + }, [derived]); + return ref; +} + +``` + + +## Error + +``` + 11 | const ref = useRef(); + 12 | const derived = cond ? ref.current : makeObject(); +> 13 | useSpecialEffect(() => { + | ^^^^^^^^^^^^^^^^^^^^^^^^ +> 14 | log(derived); + | ^^^^^^^^^^^^^^^^^ +> 15 | }, [derived]); + | ^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics.. (Bailout reason: Invariant: Expected function expression scope to exist (13:15)) (13:15) + 16 | return ref; + 17 | } + 18 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js new file mode 100644 index 0000000000000..f3a57bb912547 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js @@ -0,0 +1,17 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useRef} from 'react'; +import {useSpecialEffect} from 'shared-runtime'; + +/** + * The retry pipeline disables memoization features, which means we need to + * provide an alternate implementation of effect dependencies which does not + * rely on memoization. + */ +function useFoo({cond}) { + const ref = useRef(); + const derived = cond ? ref.current : makeObject(); + useSpecialEffect(() => { + log(derived); + }, [derived]); + return ref; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md index 86836d86b6092..c5d7456d653c2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @enableFire +// @enableFire @panicThreshold(none) import {fire} from 'react'; /** @@ -29,21 +29,13 @@ function Component({prop1}) { ## Error ``` - 9 | function Component({prop1}) { - 10 | const foo = () => { -> 11 | try { - | ^^^^^ -> 12 | console.log(prop1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -> 13 | } finally { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -> 14 | console.log('jbrown215'); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -> 15 | } - | ^^^^^^ Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (11:15) 16 | }; 17 | useEffect(() => { - 18 | fire(foo()); +> 18 | fire(foo()); + | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler. (Bailout reason: Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (11:15)) (18:18) + 19 | }); + 20 | } + 21 | ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js index ec50ed84c5780..b1ee459177509 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js @@ -1,4 +1,4 @@ -// @enableFire +// @enableFire @panicThreshold(none) import {fire} from 'react'; /** diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.expect.md new file mode 100644 index 0000000000000..ddcc86ff00efe --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.expect.md @@ -0,0 +1,23 @@ + +## Input + +```javascript +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +console.log(fire == null); + +``` + + +## Error + +``` + 2 | import {fire} from 'react'; + 3 | +> 4 | console.log(fire == null); + | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler (4:4) + 5 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.js new file mode 100644 index 0000000000000..25a60a97167dd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.js @@ -0,0 +1,4 @@ +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +console.log(fire == null); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md new file mode 100644 index 0000000000000..84a27b43b8950 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +/** + * TODO: we should eventually distinguish between `use no memo` and `use no + * compiler` directives. The former should be used to *only* disable memoization + * features. + */ +function Component({props, bar}) { + 'use no memo'; + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + fire(foo()); + fire(bar()); + }); + + return null; +} + +``` + + +## Error + +``` + 13 | }; + 14 | useEffect(() => { +> 15 | fire(foo(props)); + | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler (15:15) + 16 | fire(foo()); + 17 | fire(bar()); + 18 | }); +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.js similarity index 51% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.js index 2587e24ee1170..92e1ff211a62c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.js @@ -1,6 +1,11 @@ -// @enableFire +// @enableFire @panicThreshold(none) import {fire} from 'react'; +/** + * TODO: we should eventually distinguish between `use no memo` and `use no + * compiler` directives. The former should be used to *only* disable memoization + * features. + */ function Component({props, bar}) { 'use no memo'; const foo = () => { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md new file mode 100644 index 0000000000000..fecc28bb00158 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md @@ -0,0 +1,95 @@ + +## Input + +```javascript +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +/** + * Compilation of this file should succeed. + */ +function NonFireComponent({prop1}) { + /** + * This component bails out but does not use fire + */ + const foo = () => { + try { + console.log(prop1); + } finally { + console.log('jbrown215'); + } + }; + useEffect(() => { + foo(); + }); +} + +function FireComponent(props) { + /** + * This component uses fire and compiles successfully + */ + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + }); + + return null; +} + +``` + +## Code + +```javascript +import { useFire } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enableFire @panicThreshold(none) +import { fire } from "react"; + +/** + * Compilation of this file should succeed. + */ +function NonFireComponent({ prop1 }) { + /** + * This component bails out but does not use fire + */ + const foo = () => { + try { + console.log(prop1); + } finally { + console.log("jbrown215"); + } + }; + useEffect(() => { + foo(); + }); +} + +function FireComponent(props) { + const $ = _c(3); + + const foo = _temp; + const t0 = useFire(foo); + let t1; + if ($[0] !== props || $[1] !== t0) { + t1 = () => { + t0(props); + }; + $[0] = props; + $[1] = t0; + $[2] = t1; + } else { + t1 = $[2]; + } + useEffect(t1); + return null; +} +function _temp(props_0) { + console.log(props_0); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js new file mode 100644 index 0000000000000..899fa33376512 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js @@ -0,0 +1,35 @@ +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +/** + * Compilation of this file should succeed. + */ +function NonFireComponent({prop1}) { + /** + * This component bails out but does not use fire + */ + const foo = () => { + try { + console.log(prop1); + } finally { + console.log('jbrown215'); + } + }; + useEffect(() => { + foo(); + }); +} + +function FireComponent(props) { + /** + * This component uses fire and compiles successfully + */ + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.expect.md deleted file mode 100644 index 907501228b10b..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.expect.md +++ /dev/null @@ -1,47 +0,0 @@ - -## Input - -```javascript -// @enableFire -import {fire} from 'react'; - -function Component({props, bar}) { - 'use no memo'; - const foo = () => { - console.log(props); - }; - useEffect(() => { - fire(foo(props)); - fire(foo()); - fire(bar()); - }); - - return null; -} - -``` - -## Code - -```javascript -// @enableFire -import { fire } from "react"; - -function Component({ props, bar }) { - "use no memo"; - const foo = () => { - console.log(props); - }; - useEffect(() => { - fire(foo(props)); - fire(foo()); - fire(bar()); - }); - - return null; -} - -``` - -### Eval output -(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts index 71be6b6622eb5..ea5c30d5b27f0 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts @@ -274,6 +274,38 @@ const tests: CompilerTestCases = { }, ], }, + { + name: 'Pipeline errors are reported', + code: normalizeIndent` + import useMyEffect from 'useMyEffect'; + function Component({a}) { + 'use no memo'; + useMyEffect(() => console.log(a.b)); + return
Hello world
; + } + `, + options: [ + { + environment: { + inferEffectDependencies: [ + { + function: { + source: 'useMyEffect', + importSpecifierName: 'default', + }, + numRequiredArgs: 1, + }, + ], + }, + }, + ], + errors: [ + { + message: + '[InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics.', + }, + ], + }, ], };