From dbf2538355c288c984a58f4d417aff2396924882 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 14 Nov 2025 12:14:23 -0800 Subject: [PATCH 1/4] [compiler] Repro for false positive mutation of a value derived from props (#35139) Repro from the compiler WG (Thanks Cody!) of a case where the compiler incorrectly thinks a value is mutable. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35139). * #35140 * __->__ #35139 --- ...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 00000000000..9bd31999f62 --- /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 00000000000..bcbd85231f5 --- /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 19b769fa5f143f9c23424cd744d85e3742450235 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 14 Nov 2025 12:14:34 -0800 Subject: [PATCH 2/4] [compiler] Fix for inferring props-derived-value as mutable (#35140) 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. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35140). * __->__ #35140 * #35139 --- .../Inference/InferMutationAliasingEffects.ts | 1 + ...ure-from-prop-with-default-value.expect.md | 45 +++++++++++++++++++ ...estructure-from-prop-with-default-value.js | 13 ++++++ 3 files changed, 59 insertions(+) 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 94be2100f57..b894eb28986 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/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 00000000000..eec95683aa2 --- /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 00000000000..dbcb7303778 --- /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; +} From 647e13366cfacd4f922ef287cef7a14e4ee60f52 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:50:38 -0800 Subject: [PATCH 3/4] [compiler] fix bad rebase from sapling (#35145) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35145). * #35144 * __->__ #35145 --- ...ure-from-prop-with-default-value.expect.md | 43 ------------------- ...estructure-from-prop-with-default-value.js | 15 ------- 2 files changed, 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 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 9bd31999f62..00000000000 --- 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 bcbd85231f5..00000000000 --- 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; -} From fb2177c153353621c5e343b0386993e5084f641e Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Fri, 14 Nov 2025 23:52:11 +0100 Subject: [PATCH 4/4] [Flight] Fix pending chunks count for streams & async iterables in DEV (#35143) In DEV, we need to prevent the response from being GC'd while there are still pending chunks for ReadableSteams or pending results for AsyncIterables. Co-authored-by: Sebastian "Sebbie" Silbermann --- packages/react-client/src/ReactFlightClient.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index cc0361e6b8d..49da30ccf4c 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -813,6 +813,12 @@ function createInitializedStreamChunk< value: T, controller: FlightStreamController, ): InitializedChunk { + if (__DEV__) { + // Retain a strong reference to the Response while we wait for chunks. + if (response._pendingChunks++ === 0) { + response._weakResponse.response = response; + } + } // We use the reason field to stash the controller since we already have that // field. It's a bit of a hack but efficient. // $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors @@ -3075,7 +3081,6 @@ function resolveStream>( // We already resolved. We didn't expect to see this. return; } - releasePendingChunk(response, chunk); const resolveListeners = chunk.value; @@ -3375,6 +3380,14 @@ function stopStream( // We didn't expect not to have an existing stream; return; } + if (__DEV__) { + if (--response._pendingChunks === 0) { + // We're no longer waiting for any more chunks. We can release the strong + // reference to the response. We'll regain it if we ask for any more data + // later on. + response._weakResponse.response = null; + } + } const streamChunk: InitializedStreamChunk = (chunk: any); const controller = streamChunk.reason; controller.close(row === '' ? '"$undefined"' : row);