-
Notifications
You must be signed in to change notification settings - Fork 50.2k
[compiler][gating] Experimental directive based gating #33149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ import { | |
| CompilerErrorDetail, | ||
| ErrorSeverity, | ||
| } from '../CompilerError'; | ||
| import {ReactFunctionType} from '../HIR/Environment'; | ||
| import {ExternalFunction, ReactFunctionType} from '../HIR/Environment'; | ||
| import {CodegenFunction} from '../ReactiveScopes'; | ||
| import {isComponentDeclaration} from '../Utils/ComponentDeclaration'; | ||
| import {isHookDeclaration} from '../Utils/HookDeclaration'; | ||
|
|
@@ -31,6 +31,7 @@ import { | |
| suppressionsToCompilerError, | ||
| } from './Suppression'; | ||
| import {GeneratedSource} from '../HIR'; | ||
| import {Err, Ok, Result} from '../Utils/Result'; | ||
|
|
||
| export type CompilerPass = { | ||
| opts: PluginOptions; | ||
|
|
@@ -40,15 +41,24 @@ export type CompilerPass = { | |
| }; | ||
| export const OPT_IN_DIRECTIVES = new Set(['use forget', 'use memo']); | ||
| export const OPT_OUT_DIRECTIVES = new Set(['use no forget', 'use no memo']); | ||
| const DYNAMIC_GATING_DIRECTIVE = new RegExp('^use memo if\\(([^\\)]*)\\)$'); | ||
|
|
||
| export function findDirectiveEnablingMemoization( | ||
| export function tryFindDirectiveEnablingMemoization( | ||
| directives: Array<t.Directive>, | ||
| ): t.Directive | null { | ||
| return ( | ||
| directives.find(directive => | ||
| OPT_IN_DIRECTIVES.has(directive.value.value), | ||
| ) ?? null | ||
| opts: PluginOptions, | ||
| ): Result<t.Directive | null, CompilerError> { | ||
| const optIn = directives.find(directive => | ||
| OPT_IN_DIRECTIVES.has(directive.value.value), | ||
| ); | ||
| if (optIn != null) { | ||
| return Ok(optIn); | ||
| } | ||
| const dynamicGating = findDirectivesDynamicGating(directives, opts); | ||
| if (dynamicGating.isOk()) { | ||
| return Ok(dynamicGating.unwrap()?.directive ?? null); | ||
| } else { | ||
| return Err(dynamicGating.unwrapErr()); | ||
| } | ||
| } | ||
|
|
||
| export function findDirectiveDisablingMemoization( | ||
|
|
@@ -60,6 +70,64 @@ export function findDirectiveDisablingMemoization( | |
| ) ?? null | ||
| ); | ||
| } | ||
| function findDirectivesDynamicGating( | ||
| directives: Array<t.Directive>, | ||
| opts: PluginOptions, | ||
| ): Result< | ||
| { | ||
| gating: ExternalFunction; | ||
| directive: t.Directive; | ||
| } | null, | ||
| CompilerError | ||
| > { | ||
| if (opts.dynamicGating === null) { | ||
| return Ok(null); | ||
| } | ||
| const errors = new CompilerError(); | ||
| const result: Array<{directive: t.Directive; match: string}> = []; | ||
|
|
||
| for (const directive of directives) { | ||
| const maybeMatch = DYNAMIC_GATING_DIRECTIVE.exec(directive.value.value); | ||
| if (maybeMatch != null && maybeMatch[1] != null) { | ||
| if (t.isValidIdentifier(maybeMatch[1])) { | ||
| result.push({directive, match: maybeMatch[1]}); | ||
| } else { | ||
| errors.push({ | ||
| reason: `Dynamic gating directive is not a valid JavaScript identifier`, | ||
| description: `Found '${directive.value.value}'`, | ||
| severity: ErrorSeverity.InvalidReact, | ||
| loc: directive.loc ?? null, | ||
| suggestions: null, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| if (errors.hasErrors()) { | ||
| return Err(errors); | ||
| } else if (result.length > 1) { | ||
| const error = new CompilerError(); | ||
| error.push({ | ||
| reason: `Multiple dynamic gating directives found`, | ||
| description: `Expected a single directive but found [${result | ||
| .map(r => r.directive.value.value) | ||
| .join(', ')}]`, | ||
| severity: ErrorSeverity.InvalidReact, | ||
| loc: result[0].directive.loc ?? null, | ||
| suggestions: null, | ||
| }); | ||
| return Err(error); | ||
| } else if (result.length === 1) { | ||
| return Ok({ | ||
| gating: { | ||
| source: opts.dynamicGating.source, | ||
| importSpecifierName: result[0].match, | ||
| }, | ||
| directive: result[0].directive, | ||
| }); | ||
| } else { | ||
| return Ok(null); | ||
| } | ||
| } | ||
|
|
||
| function isCriticalError(err: unknown): boolean { | ||
| return !(err instanceof CompilerError) || err.isCritical(); | ||
|
|
@@ -325,6 +393,8 @@ export function compileProgram( | |
| filename: pass.filename, | ||
| code: pass.code, | ||
| suppressions, | ||
| hasModuleScopeOptOut: | ||
| findDirectiveDisablingMemoization(program.node.directives) != null, | ||
| }); | ||
|
|
||
| const queue: Array<CompileSource> = findFunctionsToCompile( | ||
|
|
@@ -368,7 +438,19 @@ export function compileProgram( | |
| } | ||
|
|
||
| // Avoid modifying the program if we find a program level opt-out | ||
| if (findDirectiveDisablingMemoization(program.node.directives) != null) { | ||
| if (programContext.hasModuleScopeOptOut) { | ||
| if (compiledFns.length > 0) { | ||
| const error = new CompilerError(); | ||
| error.pushErrorDetail( | ||
| new CompilerErrorDetail({ | ||
| reason: | ||
| 'Unexpected compiled functions when module scope opt-out is present', | ||
| severity: ErrorSeverity.Invariant, | ||
| loc: null, | ||
| }), | ||
| ); | ||
| handleError(error, programContext, null); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
|
|
@@ -463,12 +545,32 @@ function processFn( | |
| fnType: ReactFunctionType, | ||
| programContext: ProgramContext, | ||
| ): null | CodegenFunction { | ||
| let directives; | ||
| let directives: { | ||
| optIn: t.Directive | null; | ||
| optOut: t.Directive | null; | ||
| }; | ||
| if (fn.node.body.type !== 'BlockStatement') { | ||
| directives = {optIn: null, optOut: null}; | ||
| directives = { | ||
| optIn: null, | ||
| optOut: null, | ||
| }; | ||
| } else { | ||
| const optIn = tryFindDirectiveEnablingMemoization( | ||
| fn.node.body.directives, | ||
| programContext.opts, | ||
| ); | ||
| if (optIn.isErr()) { | ||
| /** | ||
| * If parsing opt-in directive fails, it's most likely that React Compiler | ||
| * was not tested or rolled out on this function. In that case, we handle | ||
| * the error and fall back to the safest option which is to not optimize | ||
| * the function. | ||
| */ | ||
| handleError(optIn.unwrapErr(), programContext, fn.node.loc ?? null); | ||
| return null; | ||
| } | ||
| directives = { | ||
| optIn: findDirectiveEnablingMemoization(fn.node.body.directives), | ||
| optIn: optIn.unwrapOr(null), | ||
| optOut: findDirectiveDisablingMemoization(fn.node.body.directives), | ||
| }; | ||
| } | ||
|
|
@@ -491,9 +593,10 @@ function processFn( | |
| } | ||
|
|
||
| /** | ||
| * Otherwise if 'use no forget/memo' is present, we still run the code through the compiler | ||
| * for validation but we don't mutate the babel AST. This allows us to flag if there is an | ||
| * unused 'use no forget/memo' directive. | ||
| * If 'use no forget/memo' is present and we still ran the code through the | ||
| * compiler for validation, log a skip event and don't mutate the babel AST. | ||
| * This allows us to flag if there is an unused 'use no forget/memo' | ||
| * directive. | ||
| */ | ||
| if ( | ||
| programContext.opts.ignoreUseNoForget === false && | ||
|
|
@@ -518,16 +621,7 @@ function processFn( | |
| prunedMemoValues: compiledFn.prunedMemoValues, | ||
| }); | ||
|
|
||
| /** | ||
| * Always compile functions with opt in directives. | ||
| */ | ||
| if (directives.optIn != null) { | ||
| return compiledFn; | ||
| } else if (programContext.opts.compilationMode === 'annotation') { | ||
| /** | ||
| * If no opt-in directive is found and the compiler is configured in | ||
| * annotation mode, don't insert the compiled function. | ||
| */ | ||
| if (programContext.hasModuleScopeOptOut) { | ||
| return null; | ||
| } else if (programContext.opts.noEmit) { | ||
| /** | ||
|
|
@@ -541,6 +635,15 @@ function processFn( | |
| } | ||
| } | ||
| return null; | ||
| } else if ( | ||
| programContext.opts.compilationMode === 'annotation' && | ||
| directives.optIn == null | ||
| ) { | ||
| /** | ||
| * If no opt-in directive is found and the compiler is configured in | ||
| * annotation mode, don't insert the compiled function. | ||
| */ | ||
| return null; | ||
| } else { | ||
| return compiledFn; | ||
| } | ||
|
|
@@ -644,25 +747,31 @@ function applyCompiledFunctions( | |
| pass: CompilerPass, | ||
| programContext: ProgramContext, | ||
| ): void { | ||
| const referencedBeforeDeclared = | ||
| pass.opts.gating != null | ||
| ? getFunctionReferencedBeforeDeclarationAtTopLevel(program, compiledFns) | ||
| : null; | ||
| let referencedBeforeDeclared = null; | ||
| for (const result of compiledFns) { | ||
| const {kind, originalFn, compiledFn} = result; | ||
| const transformedFn = createNewFunctionNode(originalFn, compiledFn); | ||
| programContext.alreadyCompiled.add(transformedFn); | ||
|
|
||
| if (referencedBeforeDeclared != null && kind === 'original') { | ||
| CompilerError.invariant(pass.opts.gating != null, { | ||
| reason: "Expected 'gating' import to be present", | ||
| loc: null, | ||
| }); | ||
| let dynamicGating: ExternalFunction | null = null; | ||
| if (originalFn.node.body.type === 'BlockStatement') { | ||
| const result = findDirectivesDynamicGating( | ||
| originalFn.node.body.directives, | ||
| pass.opts, | ||
| ); | ||
| if (result.isOk()) { | ||
| dynamicGating = result.unwrap()?.gating ?? null; | ||
| } | ||
| } | ||
| const functionGating = dynamicGating ?? pass.opts.gating; | ||
| if (kind === 'original' && functionGating != null) { | ||
| referencedBeforeDeclared ??= | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure how we feel about
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this is fine, |
||
| getFunctionReferencedBeforeDeclarationAtTopLevel(program, compiledFns); | ||
| insertGatedFunctionDeclaration( | ||
| originalFn, | ||
| transformedFn, | ||
| programContext, | ||
| pass.opts.gating, | ||
| functionGating, | ||
| referencedBeforeDeclared.has(result), | ||
| ); | ||
| } else { | ||
|
|
@@ -718,8 +827,13 @@ function getReactFunctionType( | |
| ): ReactFunctionType | null { | ||
| const hookPattern = pass.opts.environment.hookPattern; | ||
| if (fn.node.body.type === 'BlockStatement') { | ||
| if (findDirectiveEnablingMemoization(fn.node.body.directives) != null) | ||
| const optInDirectives = tryFindDirectiveEnablingMemoization( | ||
| fn.node.body.directives, | ||
| pass.opts, | ||
| ); | ||
| if (optInDirectives.unwrapOr(null) != null) { | ||
| return getComponentOrHookLike(fn, hookPattern) ?? 'Other'; | ||
| } | ||
| } | ||
|
|
||
| // Component and hook declarations are known components/hooks | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dynamic gating should also be an opt-in if
compilationMode: "annotation"is enabled.If
"use no memo"is set at module scope,use memo if(...)should have the same semantics asuse memo, whatever that is