Skip to content

Commit 4082b0e

Browse files
josephsavonajosephsavona
authored
[compiler] Detect known incompatible libraries (facebook#34027)
A few libraries are known to be incompatible with memoization, whether manually via `useMemo()` or via React Compiler. This puts us in a tricky situation. On the one hand, we understand that these libraries were developed prior to our documenting the [Rules of React](https://react.dev/reference/rules), and their designs were the result of trying to deliver a great experience for their users and balance multiple priorities around DX, performance, etc. At the same time, using these libraries with memoization — and in particular with automatic memoization via React Compiler — can break apps by causing the components using these APIs not to update. Concretely, the APIs have in common that they return a function which returns different values over time, but where the function itself does not change. Memoizing the result on the identity of the function will mean that the value never changes. Developers reasonable interpret this as "React Compiler broke my code". Of course, the best solution is to work with developers of these libraries to address the root cause, and we're doing that. We've previously discussed this situation with both of the respective libraries: * React Hook Form: react-hook-form/react-hook-form#11910 (comment) * TanStack Table: facebook#33057 (comment) and TanStack/table#5567 In the meantime we need to make sure that React Compiler can work out of the box as much as possible. This means teaching it about popular libraries that cannot be memoized. We also can't silently skip compilation, as this confuses users, so we need these error messages to be visible to users. To that end, this PR adds: * A flag to mark functions/hooks as incompatible * Validation against use of such functions * A default type provider to provide declarations for two known-incompatible libraries Note that Mobx is also incompatible, but the `observable()` function is called outside of the component itself, so the compiler cannot currently detect it. We may add validation for such APIs in the future. Again, we really empathize with the developers of these libraries. We've tried to word the error message non-judgementally, because we get that it's hard! We're open to feedback about the error message, please let us know.
1 parent 6b49c44 commit 4082b0e

File tree

52 files changed

+364
-55
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+364
-55
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,14 @@ export enum ErrorSeverity {
3636
* memoization.
3737
*/
3838
CannotPreserveMemoization = 'CannotPreserveMemoization',
39+
/**
40+
* An API that is known to be incompatible with the compiler. Generally as a result of
41+
* the library using "interior mutability", ie having a value whose referential identity
42+
* stays the same but which provides access to values that can change. For example a
43+
* function that doesn't change but returns different results, or an object that doesn't
44+
* change identity but whose properties change.
45+
*/
46+
IncompatibleLibrary = 'IncompatibleLibrary',
3947
/**
4048
* Unhandled syntax that we don't support yet.
4149
*/
@@ -458,7 +466,8 @@ export class CompilerError extends Error {
458466
case ErrorSeverity.InvalidJS:
459467
case ErrorSeverity.InvalidReact:
460468
case ErrorSeverity.InvalidConfig:
461-
case ErrorSeverity.UnsupportedJS: {
469+
case ErrorSeverity.UnsupportedJS:
470+
case ErrorSeverity.IncompatibleLibrary: {
462471
return true;
463472
}
464473
case ErrorSeverity.CannotPreserveMemoization:
@@ -506,8 +515,9 @@ function printErrorSummary(severity: ErrorSeverity, message: string): string {
506515
severityCategory = 'Error';
507516
break;
508517
}
518+
case ErrorSeverity.IncompatibleLibrary:
509519
case ErrorSeverity.CannotPreserveMemoization: {
510-
severityCategory = 'Memoization';
520+
severityCategory = 'Compilation Skipped';
511521
break;
512522
}
513523
case ErrorSeverity.Invariant: {
@@ -547,6 +557,9 @@ export enum ErrorCategory {
547557
// Checks that manual memoization is preserved
548558
PreserveManualMemo = 'PreserveManualMemo',
549559

560+
// Checks for known incompatible libraries
561+
IncompatibleLibrary = 'IncompatibleLibrary',
562+
550563
// Checking for no mutations of props, hook arguments, hook return values
551564
Immutability = 'Immutability',
552565

@@ -870,6 +883,15 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
870883
recommended: true,
871884
};
872885
}
886+
case ErrorCategory.IncompatibleLibrary: {
887+
return {
888+
category,
889+
name: 'incompatible-library',
890+
description:
891+
'Validates against usage of libraries which are incompatible with memoization (manual or automatic)',
892+
recommended: true,
893+
};
894+
}
873895
default: {
874896
assertExhaustive(category, `Unsupported category ${category}`);
875897
}

‎compiler/packages/babel-plugin-react-compiler/src/HIR/DefaultModuleTypeProvider.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import {Effect, ValueKind} from '..';
9+
import {TypeConfig} from './TypeSchema';
10+
11+
/**
12+
* Libraries developed before we officially documented the [Rules of React](https://react.dev/reference/rules)
13+
* implement APIs which cannot be memoized safely, either via manual or automatic memoization.
14+
*
15+
* Any non-hook API that is designed to be called during render (not events/effects) should be safe to memoize:
16+
*
17+
* ```js
18+
* function Component() {
19+
* const {someFunction} = useLibrary();
20+
* // it should always be safe to memoize functions like this
21+
* const result = useMemo(() => someFunction(), [someFunction]);
22+
* }
23+
* ```
24+
*
25+
* However, some APIs implement "interior mutability" — mutating values rather than copying into a new value
26+
* and setting state with the new value. Such functions (`someFunction()` in the example) could return different
27+
* values even though the function itself is the same object. This breaks memoization, since React relies on
28+
* the outer object (or function) changing if part of its value has changed.
29+
*
30+
* Given that we didn't have the Rules of React precisely documented prior to the introduction of React compiler,
31+
* it's understandable that some libraries accidentally shipped APIs that break this rule. However, developers
32+
* can easily run into pitfalls with these APIs. They may manually memoize them, which can break their app. Or
33+
* they may try using React Compiler, and think that the compiler has broken their code.
34+
*
35+
* To help ensure that developers can successfully use the compiler with existing code, this file teaches the
36+
* compiler about specific APIs that are known to be incompatible with memoization. We've tried to be as precise
37+
* as possible.
38+
*
39+
* The React team is open to collaborating with library authors to help develop compatible versions of these APIs,
40+
* and we have already reached out to the teams who own any API listed here to ensure they are aware of the issue.
41+
*/
42+
export function defaultModuleTypeProvider(
43+
moduleName: string,
44+
): TypeConfig | null {
45+
switch (moduleName) {
46+
case 'react-hook-form': {
47+
return {
48+
kind: 'object',
49+
properties: {
50+
useForm: {
51+
kind: 'hook',
52+
returnType: {
53+
kind: 'object',
54+
properties: {
55+
// Only the `watch()` function returned by react-hook-form's `useForm()` API is incompatible
56+
watch: {
57+
kind: 'function',
58+
positionalParams: [],
59+
restParam: Effect.Read,
60+
calleeEffect: Effect.Read,
61+
returnType: {kind: 'type', name: 'Any'},
62+
returnValueKind: ValueKind.Mutable,
63+
knownIncompatible: `React Hook Form's \`useForm()\` API returns a \`watch()\` function which cannot be memoized safely.`,
64+
},
65+
},
66+
},
67+
},
68+
},
69+
};
70+
}
71+
case '@tanstack/react-table': {
72+
return {
73+
kind: 'object',
74+
properties: {
75+
/*
76+
* Many of the properties of `useReactTable()`'s return value are incompatible, so we mark the entire hook
77+
* as incompatible
78+
*/
79+
useReactTable: {
80+
kind: 'hook',
81+
positionalParams: [],
82+
restParam: Effect.Read,
83+
returnType: {kind: 'type', name: 'Any'},
84+
knownIncompatible: `TanStack Table's \`useReactTable()\` API returns functions that cannot be memoized safely`,
85+
},
86+
},
87+
};
88+
}
89+
}
90+
return null;
91+
}

‎compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
import {Scope as BabelScope, NodePath} from '@babel/traverse';
5151
import {TypeSchema} from './TypeSchema';
5252
import {FlowTypeEnv} from '../Flood/Types';
53+
import {defaultModuleTypeProvider} from './DefaultModuleTypeProvider';
5354

5455
export const ReactElementSymbolSchema = z.object({
5556
elementSymbol: z.union([
@@ -860,10 +861,16 @@ export class Environment {
860861
#resolveModuleType(moduleName: string, loc: SourceLocation): Global | null {
861862
let moduleType = this.#moduleTypes.get(moduleName);
862863
if (moduleType === undefined) {
863-
if (this.config.moduleTypeProvider == null) {
864+
/*
865+
* NOTE: Zod doesn't work when specifying a function as a default, so we have to
866+
* fallback to the default value here
867+
*/
868+
const moduleTypeProvider =
869+
this.config.moduleTypeProvider ?? defaultModuleTypeProvider;
870+
if (moduleTypeProvider == null) {
864871
return null;
865872
}
866-
const unparsedModuleConfig = this.config.moduleTypeProvider(moduleName);
873+
const unparsedModuleConfig = moduleTypeProvider(moduleName);
867874
if (unparsedModuleConfig != null) {
868875
const parsedModuleConfig = TypeSchema.safeParse(unparsedModuleConfig);
869876
if (!parsedModuleConfig.success) {

‎compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,7 @@ export function installTypeConfig(
10011001
mutableOnlyIfOperandsAreMutable:
10021002
typeConfig.mutableOnlyIfOperandsAreMutable === true,
10031003
aliasing: typeConfig.aliasing,
1004+
knownIncompatible: typeConfig.knownIncompatible ?? null,
10041005
});
10051006
}
10061007
case 'hook': {
@@ -1019,6 +1020,7 @@ export function installTypeConfig(
10191020
returnValueKind: typeConfig.returnValueKind ?? ValueKind.Frozen,
10201021
noAlias: typeConfig.noAlias === true,
10211022
aliasing: typeConfig.aliasing,
1023+
knownIncompatible: typeConfig.knownIncompatible ?? null,
10221024
});
10231025
}
10241026
case 'object': {

‎compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ export type FunctionSignature = {
332332
mutableOnlyIfOperandsAreMutable?: boolean;
333333

334334
impure?: boolean;
335+
knownIncompatible?: string | null | undefined;
335336

336337
canonicalName?: string;
337338

‎compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ export type FunctionTypeConfig = {
251251
impure?: boolean | null | undefined;
252252
canonicalName?: string | null | undefined;
253253
aliasing?: AliasingSignatureConfig | null | undefined;
254+
knownIncompatible?: string | null | undefined;
254255
};
255256
export const FunctionTypeSchema: z.ZodType<FunctionTypeConfig> = z.object({
256257
kind: z.literal('function'),
@@ -264,6 +265,7 @@ export const FunctionTypeSchema: z.ZodType<FunctionTypeConfig> = z.object({
264265
impure: z.boolean().nullable().optional(),
265266
canonicalName: z.string().nullable().optional(),
266267
aliasing: AliasingSignatureSchema.nullable().optional(),
268+
knownIncompatible: z.string().nullable().optional(),
267269
});
268270

269271
export type HookTypeConfig = {
@@ -274,6 +276,7 @@ export type HookTypeConfig = {
274276
returnValueKind?: ValueKind | null | undefined;
275277
noAlias?: boolean | null | undefined;
276278
aliasing?: AliasingSignatureConfig | null | undefined;
279+
knownIncompatible?: string | null | undefined;
277280
};
278281
export const HookTypeSchema: z.ZodType<HookTypeConfig> = z.object({
279282
kind: z.literal('hook'),
@@ -283,6 +286,7 @@ export const HookTypeSchema: z.ZodType<HookTypeConfig> = z.object({
283286
returnValueKind: ValueKindSchema.nullable().optional(),
284287
noAlias: z.boolean().nullable().optional(),
285288
aliasing: AliasingSignatureSchema.nullable().optional(),
289+
knownIncompatible: z.string().nullable().optional(),
286290
});
287291

288292
export type BuiltInTypeConfig =

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2170,6 +2170,27 @@ function computeEffectsForLegacySignature(
21702170
}),
21712171
});
21722172
}
2173+
if (signature.knownIncompatible != null && state.env.isInferredMemoEnabled) {
2174+
const errors = new CompilerError();
2175+
errors.pushDiagnostic(
2176+
CompilerDiagnostic.create({
2177+
category: ErrorCategory.IncompatibleLibrary,
2178+
severity: ErrorSeverity.IncompatibleLibrary,
2179+
reason: 'Use of incompatible library',
2180+
description: [
2181+
'This API returns functions which cannot be memoized without leading to stale UI. ' +
2182+
'To prevent this, by default React Compiler will skip memoizing this component/hook. ' +
2183+
'However, you may see issues if values from this API are passed to other components/hooks that are ' +
2184+
'memoized.',
2185+
].join(''),
2186+
}).withDetail({
2187+
kind: 'error',
2188+
loc: receiver.loc,
2189+
message: signature.knownIncompatible,
2190+
}),
2191+
);
2192+
throw errors;
2193+
}
21732194
const stores: Array<Place> = [];
21742195
const captures: Array<Place> = [];
21752196
function visit(place: Place, effect: Effect): void {

‎compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,7 @@ function validateInferredDep(
284284
CompilerDiagnostic.create({
285285
category: ErrorCategory.PreserveManualMemo,
286286
severity: ErrorSeverity.CannotPreserveMemoization,
287-
reason:
288-
'Compilation skipped because existing memoization could not be preserved',
287+
reason: 'Existing memoization could not be preserved',
289288
description: [
290289
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. ',
291290
'The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. ',
@@ -539,8 +538,7 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
539538
CompilerDiagnostic.create({
540539
category: ErrorCategory.PreserveManualMemo,
541540
severity: ErrorSeverity.CannotPreserveMemoization,
542-
reason:
543-
'Compilation skipped because existing memoization could not be preserved',
541+
reason: 'Existing memoization could not be preserved',
544542
description: [
545543
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. ',
546544
'This dependency may be mutated later, which could cause the value to change unexpectedly.',
@@ -588,8 +586,7 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
588586
CompilerDiagnostic.create({
589587
category: ErrorCategory.PreserveManualMemo,
590588
severity: ErrorSeverity.CannotPreserveMemoization,
591-
reason:
592-
'Compilation skipped because existing memoization could not be preserved',
589+
reason: 'Existing memoization could not be preserved',
593590
description: [
594591
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output. ',
595592
DEBUG

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ function Component(props) {
2626
```
2727
Found 1 error:
2828

29-
Memoization: Compilation skipped because existing memoization could not be preserved
29+
Compilation Skipped: Existing memoization could not be preserved
3030

3131
React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source.
3232

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ function Component(props) {
2626
```
2727
Found 1 error:
2828

29-
Memoization: Compilation skipped because existing memoization could not be preserved
29+
Compilation Skipped: Existing memoization could not be preserved
3030

3131
React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source.
3232

0 commit comments

Comments
 (0)