From 0d0f7c2ad178cc546f04bf5b302d0c27609bc3cc Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Mon, 8 Apr 2024 10:55:54 -0400 Subject: [PATCH] Update ValidatePreservedManualMemoization severity to CannotPreserveMemoization We do need to fix the error location to point to the "callsite" rather than the definition of the useMemo callback, but that aside, even if the error message were perfect, it's not meant to be actionable to the user. So let's change the severity to CannotPreserveMemoization. This preserves the validation, but the eslint plugin won't report it. ghstack-source-id: 722c88922884de05e89030a7b001bd93e0a2a114 Pull Request resolved: https://github.com/facebook/react-forget/pull/2825 --- .../src/Validation/ValidatePreservedManualMemoization.ts | 2 +- ...unmemoized-callback-captured-in-context-variable.expect.md | 2 +- ...-mutated-later-via-function-preserve-memoization.expect.md | 2 +- ...property-ref-modified-later-preserve-memoization.expect.md | 2 +- .../error.false-positive-useMemo-infer-mutate-deps.expect.md | 2 +- ....invalid-useCallback-captures-reassigned-context.expect.md | 4 ++-- ...ror.todo-useCallback-captures-invalidating-value.expect.md | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts index ee1acaef69..6a4ac39756 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts @@ -439,7 +439,7 @@ class Visitor extends ReactiveFunctionVisitor { reason: "This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized", description: null, - severity: ErrorSeverity.InvalidReact, + severity: ErrorSeverity.CannotPreserveMemoization, loc: typeof instruction.loc !== "symbol" ? instruction.loc : null, suggestions: null, }); diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md index c2058f0866..29244e5796 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md @@ -53,7 +53,7 @@ export const FIXTURE_ENTRYPOINT = { 9 | const a = useHook(); 10 | // Because b is also part of that same mutable range, it can't be memoized either > 11 | const b = useMemo(() => ({}), []); - | ^^^^^^^^^^ InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (11:11) + | ^^^^^^^^^^ CannotPreserveMemoization: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (11:11) 12 | 13 | // Conditional assignment without a subsequent mutation normally doesn't create a mutable 14 | // range, but in this case we're reassigning a context variable diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md index 8ccf03f686..49884cc325 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md @@ -45,7 +45,7 @@ export const FIXTURE_ENTRYPOINT = { > 10 | ref.current.inner = event.target.value; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 11 | }); - | ^^^^ InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) + | ^^^^ CannotPreserveMemoization: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) 12 | 13 | // The ref is modified later, extending its range and preventing memoization of onChange 14 | const reset = () => { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md index a552952f3f..9776ca264f 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md @@ -42,7 +42,7 @@ export const FIXTURE_ENTRYPOINT = { > 10 | ref.current.inner = event.target.value; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 11 | }); - | ^^^^ InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) + | ^^^^ CannotPreserveMemoization: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) 12 | 13 | // The ref is modified later, extending its range and preventing memoization of onChange 14 | ref.current.inner = null; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md index c4ad418c93..5ec8d12cf2 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md @@ -36,7 +36,7 @@ export const FIXTURE_ENTRYPOINT = { > 13 | return identity(val); | ^^^^^^^^^^^^^^^^^^^^^^^^^ > 14 | }, [val]); - | ^^^^ InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (12:14) + | ^^^^ CannotPreserveMemoization: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (12:14) 15 | } 16 | 17 | export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md index 8eed3cc790..edfed94438 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md @@ -33,9 +33,9 @@ export const FIXTURE_ENTRYPOINT = { 10 | 11 | // makeArray() is captured, but depsList contains [props] > 12 | const cb = useCallback(() => [x], [x]); - | ^^^^^^^^^ InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (12:12) + | ^^^^^^^^^ CannotPreserveMemoization: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (12:12) -InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (12:12) +CannotPreserveMemoization: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (12:12) 13 | 14 | x = makeArray(); 15 | diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md index ce0df1a629..05d3bac71b 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md @@ -31,7 +31,7 @@ export const FIXTURE_ENTRYPOINT = { 11 | x.push(props); 12 | > 13 | return useCallback(() => [x], [x]); - | ^^^^^^^^^ InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (13:13) + | ^^^^^^^^^ CannotPreserveMemoization: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (13:13) 14 | } 15 | 16 | export const FIXTURE_ENTRYPOINT = {