From 8deecf508559f064f9c1d1bf2b82ec0d90c9b4d0 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 14 Nov 2025 11:23:42 -0800 Subject: [PATCH 1/2] [compiler] Repro for false positive mutation of a value derived from props Repro from the compiler WG (Thanks Cody!) of a case where the compiler incorrectly thinks a value is mutable. --- ...ure-from-prop-with-default-value.expect.md | 43 +++++++++++++++++++ ...estructure-from-prop-with-default-value.js | 15 +++++++ 2 files changed, 58 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.expect.md new file mode 100644 index 0000000000000..9bd31999f6216 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +export function useFormatRelativeTime(opts = {}) { + const {timeZone, minimal} = opts; + const format = useCallback(function formatWithUnit() {}, [minimal]); + // We record `{timeZone}` as capturing timeZone into the object, + // then assume that dateTimeFormat() mutates that object, + // which in turn can mutate timeZone and the object it came from, + // which means that the value `minimal` is derived from can change. + // + // The bug is that we shouldn't be recording a Capture effect given + // that `opts` is known maybe-frozen. If we correctly recorded + // an ImmutableCapture, this wouldn't be mistaken as mutating + // opts/minimal + dateTimeFormat({timeZone}); + return format; +} + +``` + + +## Error + +``` +Found 1 error: + +Compilation Skipped: Existing memoization could not be preserved + +React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly. + +error.todo-repro-destructure-from-prop-with-default-value.ts:3:60 + 1 | export function useFormatRelativeTime(opts = {}) { + 2 | const {timeZone, minimal} = opts; +> 3 | const format = useCallback(function formatWithUnit() {}, [minimal]); + | ^^^^^^^ This dependency may be modified later + 4 | // We record `{timeZone}` as capturing timeZone into the object, + 5 | // then assume that dateTimeFormat() mutates that object, + 6 | // which in turn can mutate timeZone and the object it came from, +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.js new file mode 100644 index 0000000000000..bcbd85231f599 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.js @@ -0,0 +1,15 @@ +export function useFormatRelativeTime(opts = {}) { + const {timeZone, minimal} = opts; + const format = useCallback(function formatWithUnit() {}, [minimal]); + // We record `{timeZone}` as capturing timeZone into the object, + // then assume that dateTimeFormat() mutates that object, + // which in turn can mutate timeZone and the object it came from, + // which means that the value `minimal` is derived from can change. + // + // The bug is that we shouldn't be recording a Capture effect given + // that `opts` is known maybe-frozen. If we correctly recorded + // an ImmutableCapture, this wouldn't be mistaken as mutating + // opts/minimal + dateTimeFormat({timeZone}); + return format; +} From 4211a7c12f76729b2e6fcb3578176971a5e2c99c Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 14 Nov 2025 11:30:56 -0800 Subject: [PATCH 2/2] [compiler] Fix for inferring props-derived-value as mutable Fix for the repro from the previous PR. A `Capture x -> y` effect should downgrade to `ImmutableCapture` when the source value is maybe-frozen. MaybeFrozen represents the union of a frozen value with a non-frozen value. --- .../Inference/InferMutationAliasingEffects.ts | 1 + ...ure-from-prop-with-default-value.expect.md | 43 ------------------ ...estructure-from-prop-with-default-value.js | 15 ------- ...ure-from-prop-with-default-value.expect.md | 45 +++++++++++++++++++ ...estructure-from-prop-with-default-value.js | 13 ++++++ 5 files changed, 59 insertions(+), 58 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-destructure-from-prop-with-default-value.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-destructure-from-prop-with-default-value.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 94be2100f57e8..b894eb2898641 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -954,6 +954,7 @@ function applyEffect( case ValueKind.Primitive: { break; } + case ValueKind.MaybeFrozen: case ValueKind.Frozen: { sourceType = 'frozen'; break; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.expect.md deleted file mode 100644 index 9bd31999f6216..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.expect.md +++ /dev/null @@ -1,43 +0,0 @@ - -## Input - -```javascript -export function useFormatRelativeTime(opts = {}) { - const {timeZone, minimal} = opts; - const format = useCallback(function formatWithUnit() {}, [minimal]); - // We record `{timeZone}` as capturing timeZone into the object, - // then assume that dateTimeFormat() mutates that object, - // which in turn can mutate timeZone and the object it came from, - // which means that the value `minimal` is derived from can change. - // - // The bug is that we shouldn't be recording a Capture effect given - // that `opts` is known maybe-frozen. If we correctly recorded - // an ImmutableCapture, this wouldn't be mistaken as mutating - // opts/minimal - dateTimeFormat({timeZone}); - return format; -} - -``` - - -## Error - -``` -Found 1 error: - -Compilation Skipped: Existing memoization could not be preserved - -React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly. - -error.todo-repro-destructure-from-prop-with-default-value.ts:3:60 - 1 | export function useFormatRelativeTime(opts = {}) { - 2 | const {timeZone, minimal} = opts; -> 3 | const format = useCallback(function formatWithUnit() {}, [minimal]); - | ^^^^^^^ This dependency may be modified later - 4 | // We record `{timeZone}` as capturing timeZone into the object, - 5 | // then assume that dateTimeFormat() mutates that object, - 6 | // which in turn can mutate timeZone and the object it came from, -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.js deleted file mode 100644 index bcbd85231f599..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.todo-repro-destructure-from-prop-with-default-value.js +++ /dev/null @@ -1,15 +0,0 @@ -export function useFormatRelativeTime(opts = {}) { - const {timeZone, minimal} = opts; - const format = useCallback(function formatWithUnit() {}, [minimal]); - // We record `{timeZone}` as capturing timeZone into the object, - // then assume that dateTimeFormat() mutates that object, - // which in turn can mutate timeZone and the object it came from, - // which means that the value `minimal` is derived from can change. - // - // The bug is that we shouldn't be recording a Capture effect given - // that `opts` is known maybe-frozen. If we correctly recorded - // an ImmutableCapture, this wouldn't be mistaken as mutating - // opts/minimal - dateTimeFormat({timeZone}); - return format; -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-destructure-from-prop-with-default-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-destructure-from-prop-with-default-value.expect.md new file mode 100644 index 0000000000000..eec95683aa2a3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-destructure-from-prop-with-default-value.expect.md @@ -0,0 +1,45 @@ + +## Input + +```javascript +export function useFormatRelativeTime(opts = {}) { + const {timeZone, minimal} = opts; + const format = useCallback(function formatWithUnit() {}, [minimal]); + // We previously recorded `{timeZone}` as capturing timeZone into the object, + // then assumed that dateTimeFormat() mutates that object, + // which in turn could mutate timeZone and the object it came from, + // which meanteans that the value `minimal` is derived from can change. + // + // The fix was to record a Capture from a maybefrozen value as an ImmutableCapture + // which doesn't propagate mutations + dateTimeFormat({timeZone}); + return format; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +export function useFormatRelativeTime(t0) { + const $ = _c(1); + const opts = t0 === undefined ? {} : t0; + const { timeZone, minimal } = opts; + let t1; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t1 = function formatWithUnit() {}; + $[0] = t1; + } else { + t1 = $[0]; + } + const format = t1; + + dateTimeFormat({ timeZone }); + return format; +} + +``` + +### 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/new-mutability/repro-destructure-from-prop-with-default-value.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-destructure-from-prop-with-default-value.js new file mode 100644 index 0000000000000..dbcb7303778c3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-destructure-from-prop-with-default-value.js @@ -0,0 +1,13 @@ +export function useFormatRelativeTime(opts = {}) { + const {timeZone, minimal} = opts; + const format = useCallback(function formatWithUnit() {}, [minimal]); + // We previously recorded `{timeZone}` as capturing timeZone into the object, + // then assumed that dateTimeFormat() mutates that object, + // which in turn could mutate timeZone and the object it came from, + // which meanteans that the value `minimal` is derived from can change. + // + // The fix was to record a Capture from a maybefrozen value as an ImmutableCapture + // which doesn't propagate mutations + dateTimeFormat({timeZone}); + return format; +}