Skip to content

Commit

Permalink
[compiler][rfc] Enable hook guards in dev mode by default
Browse files Browse the repository at this point in the history
This validation ensures that React compiler-enabled apps remain correct. That is, code that errors with this validation is most likely ***invalid*** with React compiler is enabled (specifically, hook calls will be compiled to if-else memo blocks).

Hook guards are used extensively for Meta's react compiler rollouts. There, they're enabled for developers (for dev builds) and on e2e test runs. Let's enable by default for oss as well

### Examples of inputs this rule throws on

* Components should not be invoked directly as React Compiler could memoize the call to AnotherComponent, which introduces conditional hook calls in its compiled output.
  ```js
  function Invalid1(props) {
   const myJsx = AnotherComponent(props);
   return <div> { myJsx } </div>;
  }
  ```
* Hooks must be named as hooks. Similarly, hook calls may not appear in functions that are not components or hooks.
  ```js
  const renamedHook = useState;
  function Invalid2() {
    const [state, setState] = renamedHook(0);
  }

  function Invalid3() {
    const myFunc = () => useContext(...);
    myFunc();
  }
  ```

* Hooks must be directly called (from the body of a component or hook)
  ```
  function call(fn) {
    return fn();
  }

  function Invalid4() {
    const result = call(useMyHook);
  }
  ```
  • Loading branch information
mofeiZ committed Mar 5, 2025
1 parent e9252bc commit e7af2a5
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 18 deletions.
1 change: 1 addition & 0 deletions compiler/packages/babel-plugin-react-compiler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"pretty-format": "^24",
"react": "0.0.0-experimental-4beb1fd8-20241118",
"react-dom": "0.0.0-experimental-4beb1fd8-20241118",
"react-compiler-runtime": "0.0.1",
"ts-jest": "^29.1.1",
"ts-node": "^10.9.2",
"zod": "^3.22.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const e2eTransformerCacheKey = 1;
const forgetOptions: EnvironmentConfig = validateEnvironmentConfig({
enableAssumeHooksFollowRulesOfReact: true,
enableFunctionOutlining: false,
enableEmitHookGuards: null,
});
const debugMode = process.env['DEBUG_FORGET_COMPILER'] != null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,18 @@ export default function BabelPluginReactCompiler(
},
};
}
if (opts.environment.enableEmitHookGuards != null) {
const enableEmitHookGuards = opts.environment.enableEmitHookGuards;
if (enableEmitHookGuards.devonly === true && !isDev) {
opts = {
...opts,
environment: {
...opts.environment,
enableEmitHookGuards: null,
},
};
}
}
compileProgram(prog, {
opts,
filename: pass.filename ?? null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,27 @@ const EnvironmentConfigSchema = z.object({
*/
enableEmitFreeze: ExternalFunctionSchema.nullable().default(null),

enableEmitHookGuards: ExternalFunctionSchema.nullable().default(null),
enableEmitHookGuards: z
.intersection(
ExternalFunctionSchema,
z.object({
devonly: z.union([
z.literal(true),
z.literal(false),
/**
* Backwards compatibility with previous configuration, which did not
* have this field.
*/
z.literal(undefined),
]),
}),
)
.nullable()
.default({
source: 'react-compiler-runtime',
importSpecifierName: '$dispatcherGuard',
devonly: true,
}),

/**
* Enable instruction reordering. See InstructionReordering.ts for the details
Expand Down Expand Up @@ -668,6 +688,7 @@ const testComplexConfigDefaults: PartialEnvironmentConfig = {
enableEmitHookGuards: {
source: 'react-compiler-runtime',
importSpecifierName: '$dispatcherGuard',
devonly: false,
},
inlineJsxTransform: {
elementSymbol: 'react.transitional.element',
Expand Down Expand Up @@ -711,6 +732,7 @@ function parseConfigPragmaEnvironmentForTest(
const maybeConfig: any = {};
// Get the defaults to programmatically check for boolean properties
const defaultConfig = EnvironmentConfigSchema.parse({});
(maybeConfig as PartialEnvironmentConfig).enableEmitHookGuards = null;

for (const token of pragma.split(' ')) {
if (!token.startsWith('@')) {
Expand Down
74 changes: 57 additions & 17 deletions compiler/packages/react-compiler-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export const c =

const LazyGuardDispatcher: {[key: string]: (...args: Array<any>) => any} = {};
[
'readContext',
'useCallback',
'useContext',
'useEffect',
Expand All @@ -58,10 +57,8 @@ const LazyGuardDispatcher: {[key: string]: (...args: Array<any>) => any} = {};
'useMutableSource',
'useSyncExternalStore',
'useId',
'unstable_isNewReconciler',
'getCacheSignal',
'getCacheForType',
'useCacheRefresh',
'useOptimistic',
].forEach(name => {
LazyGuardDispatcher[name] = () => {
throw new Error(
Expand All @@ -74,15 +71,29 @@ const LazyGuardDispatcher: {[key: string]: (...args: Array<any>) => any} = {};
let originalDispatcher: unknown = null;

// Allow guards are not emitted for useMemoCache
LazyGuardDispatcher['useMemoCache'] = (count: number) => {
if (originalDispatcher == null) {
throw new Error(
'React Compiler internal invariant violation: unexpected null dispatcher',
);
} else {
return (originalDispatcher as any).useMemoCache(count);
}
};

for (const key of [
// Allow guards may not be emitted for useMemoCache
'useMemoCache',
'unstable_useMemoCache',
// Not named as hooks
'readContext',
'unstable_isNewReconciler',
'getCacheSignal',
'getCacheForType',
// Not 'real' hooks (i.e. not implemented with an index based id)
'use',
]) {
LazyGuardDispatcher[key] = (...args) => {
if (originalDispatcher == null) {
throw new Error(
'React Compiler internal invariant violation: unexpected null dispatcher',
);
} else {
return (originalDispatcher as any).useMemoCache(...args);
}
};
}

enum GuardKind {
PushGuardContext = 0,
Expand All @@ -92,8 +103,13 @@ enum GuardKind {
}

function setCurrent(newDispatcher: any) {
ReactSecretInternals.ReactCurrentDispatcher.current = newDispatcher;
return ReactSecretInternals.ReactCurrentDispatcher.current;
if (ReactSecretInternals.ReactCurrentDispatcher != null) {
ReactSecretInternals.ReactCurrentDispatcher.current = newDispatcher;
return ReactSecretInternals.ReactCurrentDispatcher.current;
} else {
ReactSecretInternals.H = newDispatcher;
return ReactSecretInternals.H;
}
}

const guardFrames: Array<unknown> = [];
Expand Down Expand Up @@ -134,7 +150,9 @@ const guardFrames: Array<unknown> = [];
* ```
*/
export function $dispatcherGuard(kind: GuardKind) {
const curr = ReactSecretInternals.ReactCurrentDispatcher.current;
const curr =
ReactSecretInternals.H ??
ReactSecretInternals.ReactCurrentDispatcher.current;
if (kind === GuardKind.PushGuardContext) {
// Push before checking invariant or errors
guardFrames.push(curr);
Expand Down Expand Up @@ -169,7 +187,29 @@ export function $dispatcherGuard(kind: GuardKind) {
// ExpectHooks could be nested, so we save the current dispatcher
// for the matching PopExpectHook to restore.
guardFrames.push(curr);
setCurrent(originalDispatcher);
if (originalDispatcher != null) {
/**
* originalDispatcher could be null in the following case.
* ```js
* function Component() {
* "use no memo";
* const useFn = useMakeHook();
* useFn();
* }
* function useMakeHook() {
* // ...
* return () => {
* useState(...);
* };
* }
* ```
*
* React compiler currently does not memoize within inner functions. While this
* code breaks the programming model (hooks as first class values), let's
* either ignore this or error with a better message (within another dispatcher)
*/
setCurrent(originalDispatcher);
}
} else if (kind === GuardKind.PopExpectHook) {
const lastFrame = guardFrames.pop();
if (lastFrame == null) {
Expand Down

0 comments on commit e7af2a5

Please sign in to comment.