From 3ec88e797f7352c87cb79292b02fdfc53050133f Mon Sep 17 00:00:00 2001 From: Stephen Zhou <38493346+hyoban@users.noreply.github.com> Date: Mon, 5 May 2025 23:37:06 +0800 Subject: [PATCH 1/6] [eslint-plugin-react-hooks] update doc url for rules of hooks (#33118) --- packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts index 6be40df53226a..ac7d0f3a06cfe 100644 --- a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts +++ b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts @@ -128,7 +128,7 @@ const rule = { docs: { description: 'enforces the Rules of Hooks', recommended: true, - url: 'https://reactjs.org/docs/hooks-rules.html', + url: 'https://react.dev/reference/rules/rules-of-hooks', }, }, create(context: Rule.RuleContext) { From 52ea641449570bbc32eb90fb1a76740249b6bcf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 5 May 2025 11:37:39 -0400 Subject: [PATCH 2/6] [Flight] Don't increase serializedSize for every recursive pass (#33123) I noticed that we increase this in the recursive part of the algorithm. This would mean that we'd count a key more than once if it has Server Components inside it recursively resolving. This moves it out to where we enter from toJSON. Which is called once per JSON entry (and therefore once per key). --- packages/react-server/src/ReactFlightServer.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index aefcf5f6ee809..0fcd8af7cbcdd 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -2302,6 +2302,9 @@ function renderModel( key: string, value: ReactClientValue, ): ReactJSONValue { + // First time we're serializing the key, we should add it to the size. + serializedSize += key.length; + const prevKeyPath = task.keyPath; const prevImplicitSlot = task.implicitSlot; try { @@ -2416,8 +2419,6 @@ function renderModelDestructive( // Set the currently rendering model task.model = value; - serializedSize += parentPropertyName.length; - // Special Symbol, that's very common. if (value === REACT_ELEMENT_TYPE) { return '$'; From 0c1575cee8a78dd097edcafc307522ad000e372c Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 5 May 2025 11:45:58 -0400 Subject: [PATCH 3/6] [compiler][bugfix] Bail out when a memo block declares hoisted fns (#32765) Note that bailing out adds false positives for hoisted functions whose only references are within other functions. For example, this rewrite would be safe. ```js // source program function foo() { return bar(); } function bar() { return 42; } // compiler output let bar; if (/* deps changed */) { function foo() { return bar(); } bar = function bar() { return 42; } } ``` These false positives are difficult to detect because any maybe-call of foo before the definition of bar would be invalid. Instead of bailing out, we should rewrite hoisted function declarations to the following form. ```js let bar$0; if (/* deps changed */) { // All references within the declaring memo block // or before the function declaration should use // the original identifier `bar` function foo() { return bar(); } function bar() { return 42; } bar$0 = bar; } // All references after the declaring memo block // or after the function declaration should use // the rewritten declaration `bar$0` ``` --- .../ReactiveScopes/PruneHoistedContexts.ts | 96 ++++++++++++++++++- .../bug-functiondecl-hoisting.expect.md | 79 --------------- ...error.todo-functiondecl-hoisting.expect.md | 43 +++++++++ ...x => error.todo-functiondecl-hoisting.tsx} | 0 ...todo-valid-functiondecl-hoisting.expect.md | 46 +++++++++ ...error.todo-valid-functiondecl-hoisting.tsx | 25 +++++ 6 files changed, 206 insertions(+), 83 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-functiondecl-hoisting.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{bug-functiondecl-hoisting.tsx => error.todo-functiondecl-hoisting.tsx} (100%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts index 66b1e1ce152b9..ae3ff122a23a2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts @@ -5,10 +5,13 @@ * LICENSE file in the root directory of this source tree. */ +import {CompilerError} from '..'; import { convertHoistedLValueKind, IdentifierId, + InstructionId, InstructionKind, + Place, ReactiveFunction, ReactiveInstruction, ReactiveScopeBlock, @@ -24,15 +27,38 @@ import { /* * Prunes DeclareContexts lowered for HoistedConsts, and transforms any references back to its * original instruction kind. + * + * Also detects and bails out on context variables which are: + * - function declarations, which are hoisted by JS engines to the nearest block scope + * - referenced before they are defined (i.e. having a `DeclareContext HoistedConst`) + * - declared + * + * This is because React Compiler converts a `function foo()` function declaration to + * 1. a `let foo;` declaration before reactive memo blocks + * 2. a `foo = function foo() {}` assignment within the block + * + * This means references before the assignment are invalid (see fixture + * error.todo-functiondecl-hoisting) */ export function pruneHoistedContexts(fn: ReactiveFunction): void { visitReactiveFunction(fn, new Visitor(), { activeScopes: empty(), + uninitialized: new Map(), }); } type VisitorState = { activeScopes: Stack>; + uninitialized: Map< + IdentifierId, + | { + kind: 'unknown-kind'; + } + | { + kind: 'func'; + definition: Place | null; + } + >; }; class Visitor extends ReactiveFunctionTransform { @@ -40,15 +66,39 @@ class Visitor extends ReactiveFunctionTransform { state.activeScopes = state.activeScopes.push( new Set(scope.scope.declarations.keys()), ); + /** + * Add declared but not initialized / assigned variables. This may include + * function declarations that escape the memo block. + */ + for (const decl of scope.scope.declarations.values()) { + state.uninitialized.set(decl.identifier.id, {kind: 'unknown-kind'}); + } this.traverseScope(scope, state); state.activeScopes.pop(); + for (const decl of scope.scope.declarations.values()) { + state.uninitialized.delete(decl.identifier.id); + } + } + override visitPlace( + _id: InstructionId, + place: Place, + state: VisitorState, + ): void { + const maybeHoistedFn = state.uninitialized.get(place.identifier.id); + if ( + maybeHoistedFn?.kind === 'func' && + maybeHoistedFn.definition !== place + ) { + CompilerError.throwTodo({ + reason: '[PruneHoistedContexts] Rewrite hoisted function references', + loc: place.loc, + }); + } } override transformInstruction( instruction: ReactiveInstruction, state: VisitorState, ): Transformed { - this.visitInstruction(instruction, state); - /** * Remove hoisted declarations to preserve TDZ */ @@ -57,6 +107,18 @@ class Visitor extends ReactiveFunctionTransform { instruction.value.lvalue.kind, ); if (maybeNonHoisted != null) { + if ( + maybeNonHoisted === InstructionKind.Function && + state.uninitialized.has(instruction.value.lvalue.place.identifier.id) + ) { + state.uninitialized.set( + instruction.value.lvalue.place.identifier.id, + { + kind: 'func', + definition: null, + }, + ); + } return {kind: 'remove'}; } } @@ -65,7 +127,7 @@ class Visitor extends ReactiveFunctionTransform { instruction.value.lvalue.kind !== InstructionKind.Reassign ) { /** - * Rewrite StoreContexts let/const/functions that will be pre-declared in + * Rewrite StoreContexts let/const that will be pre-declared in * codegen to reassignments. */ const lvalueId = instruction.value.lvalue.place.identifier.id; @@ -73,10 +135,36 @@ class Visitor extends ReactiveFunctionTransform { scope.has(lvalueId), ); if (isDeclaredByScope) { - instruction.value.lvalue.kind = InstructionKind.Reassign; + if ( + instruction.value.lvalue.kind === InstructionKind.Let || + instruction.value.lvalue.kind === InstructionKind.Const + ) { + instruction.value.lvalue.kind = InstructionKind.Reassign; + } else if (instruction.value.lvalue.kind === InstructionKind.Function) { + const maybeHoistedFn = state.uninitialized.get(lvalueId); + if (maybeHoistedFn != null) { + CompilerError.invariant(maybeHoistedFn.kind === 'func', { + reason: '[PruneHoistedContexts] Unexpected hoisted function', + loc: instruction.loc, + }); + maybeHoistedFn.definition = instruction.value.lvalue.place; + /** + * References to hoisted functions are now "safe" as variable assignments + * have finished. + */ + state.uninitialized.delete(lvalueId); + } + } else { + CompilerError.throwTodo({ + reason: '[PruneHoistedContexts] Unexpected kind', + description: `(${instruction.value.lvalue.kind})`, + loc: instruction.loc, + }); + } } } + this.visitInstruction(instruction, state); return {kind: 'keep'}; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md deleted file mode 100644 index f8712ed7289a9..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md +++ /dev/null @@ -1,79 +0,0 @@ - -## Input - -```javascript -import {Stringify} from 'shared-runtime'; - -/** - * Fixture currently fails with - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok)
{"result":{"value":2},"fn":{"kind":"Function","result":{"value":2}},"shouldInvokeFns":true}
- * Forget: - * (kind: exception) bar is not a function - */ -function Foo({value}) { - const result = bar(); - function bar() { - return {value}; - } - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{value: 2}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { Stringify } from "shared-runtime"; - -/** - * Fixture currently fails with - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok)
{"result":{"value":2},"fn":{"kind":"Function","result":{"value":2}},"shouldInvokeFns":true}
- * Forget: - * (kind: exception) bar is not a function - */ -function Foo(t0) { - const $ = _c(6); - const { value } = t0; - let bar; - let result; - if ($[0] !== value) { - result = bar(); - bar = function bar() { - return { value }; - }; - $[0] = value; - $[1] = bar; - $[2] = result; - } else { - bar = $[1]; - result = $[2]; - } - let t1; - if ($[3] !== bar || $[4] !== result) { - t1 = ; - $[3] = bar; - $[4] = result; - $[5] = t1; - } else { - t1 = $[5]; - } - return t1; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{ value: 2 }], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-functiondecl-hoisting.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-functiondecl-hoisting.expect.md new file mode 100644 index 0000000000000..6e2310bd1064c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-functiondecl-hoisting.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Fixture currently fails with + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok)
{"result":{"value":2},"fn":{"kind":"Function","result":{"value":2}},"shouldInvokeFns":true}
+ * Forget: + * (kind: exception) bar is not a function + */ +function Foo({value}) { + const result = bar(); + function bar() { + return {value}; + } + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{value: 2}], +}; + +``` + + +## Error + +``` + 10 | */ + 11 | function Foo({value}) { +> 12 | const result = bar(); + | ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (12:12) + 13 | function bar() { + 14 | return {value}; + 15 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-functiondecl-hoisting.tsx similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.tsx rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-functiondecl-hoisting.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.expect.md new file mode 100644 index 0000000000000..fb8988c8f878d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; +/** + * Also see error.todo-functiondecl-hoisting.tsx which shows *invalid* + * compilation cases. + * + * This bailout specifically is a false positive for since this function's only + * reference-before-definition are within other functions which are not invoked. + */ +function Foo() { + 'use memo'; + + function foo() { + return bar(); + } + function bar() { + return 42; + } + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [], +}; + +``` + + +## Error + +``` + 13 | return bar(); + 14 | } +> 15 | function bar() { + | ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (15:15) + 16 | return 42; + 17 | } + 18 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.tsx new file mode 100644 index 0000000000000..063492b89cf52 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.tsx @@ -0,0 +1,25 @@ +import {Stringify} from 'shared-runtime'; +/** + * Also see error.todo-functiondecl-hoisting.tsx which shows *invalid* + * compilation cases. + * + * This bailout specifically is a false positive for since this function's only + * reference-before-definition are within other functions which are not invoked. + */ +function Foo() { + 'use memo'; + + function foo() { + return bar(); + } + function bar() { + return 42; + } + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [], +}; From c129c2424b662a371865a0145c562a1cf934b023 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 5 May 2025 11:52:45 -0400 Subject: [PATCH 4/6] [compiler][repro] Nested fbt test fixture (#32779) Ideally we should detect and bail out on this case to avoid babel build failures. --- .../error.todo-fbt-param-nested-fbt.expect.md | 56 +++++++++++++++++++ .../fbt/error.todo-fbt-param-nested-fbt.js | 38 +++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.expect.md new file mode 100644 index 0000000000000..eb8073282efa1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.expect.md @@ -0,0 +1,56 @@ + +## Input + +```javascript +import fbt from 'fbt'; +import {Stringify} from 'shared-runtime'; + +/** + * MemoizeFbtAndMacroOperands needs to account for nested fbt calls. + * Expected fixture `fbt-param-call-arguments` to succeed but it failed with error: + * /fbt-param-call-arguments.ts: Line 19 Column 11: fbt: unsupported babel node: Identifier + * --- + * t3 + * --- + */ +function Component({firstname, lastname}) { + 'use memo'; + return ( + + {fbt( + [ + 'Name: ', + fbt.param('firstname', ), + ', ', + fbt.param( + 'lastname', + + {fbt('(inner fbt)', 'Inner fbt value')} + + ), + ], + 'Name' + )} + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{firstname: 'first', lastname: 'last'}], + sequentialRenders: [{firstname: 'first', lastname: 'last'}], +}; + +``` + + +## Error + +``` +Line 19 Column 11: fbt: unsupported babel node: Identifier +--- +t3 +--- +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.js new file mode 100644 index 0000000000000..14e3278e395de --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.js @@ -0,0 +1,38 @@ +import fbt from 'fbt'; +import {Stringify} from 'shared-runtime'; + +/** + * MemoizeFbtAndMacroOperands needs to account for nested fbt calls. + * Expected fixture `fbt-param-call-arguments` to succeed but it failed with error: + * /fbt-param-call-arguments.ts: Line 19 Column 11: fbt: unsupported babel node: Identifier + * --- + * t3 + * --- + */ +function Component({firstname, lastname}) { + 'use memo'; + return ( + + {fbt( + [ + 'Name: ', + fbt.param('firstname', ), + ', ', + fbt.param( + 'lastname', + + {fbt('(inner fbt)', 'Inner fbt value')} + + ), + ], + 'Name' + )} + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{firstname: 'first', lastname: 'last'}], + sequentialRenders: [{firstname: 'first', lastname: 'last'}], +}; From b9cfa0d3083f80bdd11ba76a55aa08fa659b7359 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Mon, 5 May 2025 18:30:33 +0200 Subject: [PATCH 5/6] [Flight] Prevent serialized size leaking across requests (#33121) --- .../src/__tests__/ReactFlightDOMEdge-test.js | 38 +++++++++++++++++++ .../react-server/src/ReactFlightServer.js | 23 ++++++----- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js index 5194913d2cb32..a4418b701716a 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js @@ -692,6 +692,44 @@ describe('ReactFlightDOMEdge', () => { expect(html).toBe(html2); }); + it('regression: should not leak serialized size', async () => { + const MAX_ROW_SIZE = 3200; + // This test case is a bit convoluted and may no longer trigger the original bug. + // Originally, the size of `promisedText` was not cleaned up so the sync portion + // ended up being deferred immediately when we called `renderToReadableStream` again + // i.e. `result2.syncText` became a Lazy element on the second request. + const longText = 'd'.repeat(MAX_ROW_SIZE); + const promisedText = Promise.resolve(longText); + const model = {syncText:

{longText}

, promisedText}; + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(model), + ); + + const result = await ReactServerDOMClient.createFromReadableStream(stream, { + serverConsumerManifest: { + moduleMap: null, + moduleLoading: null, + }, + }); + + const stream2 = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(model), + ); + + const result2 = await ReactServerDOMClient.createFromReadableStream( + stream2, + { + serverConsumerManifest: { + moduleMap: null, + moduleLoading: null, + }, + }, + ); + + expect(result2.syncText).toEqual(result.syncText); + }); + it('should be able to serialize any kind of typed array', async () => { const buffer = new Uint8Array([ 123, 4, 10, 5, 100, 255, 244, 45, 56, 67, 43, 124, 67, 89, 100, 20, diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 0fcd8af7cbcdd..98b17cc92088e 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -3927,18 +3927,9 @@ function emitChunk( return; } // For anything else we need to try to serialize it using JSON. - // We stash the outer parent size so we can restore it when we exit. - const parentSerializedSize = serializedSize; - // We don't reset the serialized size counter from reentry because that indicates that we - // are outlining a model and we actually want to include that size into the parent since - // it will still block the parent row. It only restores to zero at the top of the stack. - try { - // $FlowFixMe[incompatible-type] stringify can return null for undefined but we never do - const json: string = stringify(value, task.toJSON); - emitModelChunk(request, task.id, json); - } finally { - serializedSize = parentSerializedSize; - } + // $FlowFixMe[incompatible-type] stringify can return null for undefined but we never do + const json: string = stringify(value, task.toJSON); + emitModelChunk(request, task.id, json); } function erroredTask(request: Request, task: Task, error: mixed): void { @@ -3976,6 +3967,11 @@ function retryTask(request: Request, task: Task): void { const prevDebugID = debugID; task.status = RENDERING; + // We stash the outer parent size so we can restore it when we exit. + const parentSerializedSize = serializedSize; + // We don't reset the serialized size counter from reentry because that indicates that we + // are outlining a model and we actually want to include that size into the parent since + // it will still block the parent row. It only restores to zero at the top of the stack. try { // Track the root so we know that we have to emit this object even though it // already has an ID. This is needed because we might see this object twice @@ -4087,6 +4083,7 @@ function retryTask(request: Request, task: Task): void { if (__DEV__) { debugID = prevDebugID; } + serializedSize = parentSerializedSize; } } @@ -4099,9 +4096,11 @@ function tryStreamTask(request: Request, task: Task): void { // so that we instead outline the row to get a new debugID if needed. debugID = null; } + const parentSerializedSize = serializedSize; try { emitChunk(request, task, task.model); } finally { + serializedSize = parentSerializedSize; if (__DEV__) { debugID = prevDebugID; } From edf550b67936f2c62534ad5549bf580a4f581bd8 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 5 May 2025 13:36:44 -0400 Subject: [PATCH 6/6] Ship enableFabricCompleteRootInCommitPhase (#33064) This was shipped internally. Cleaning up the flag. --- .../src/ReactFiberConfigFabric.js | 10 ++-------- packages/shared/ReactFeatureFlags.js | 5 ----- .../forks/ReactFeatureFlags.native-fb-dynamic.js | 1 - packages/shared/forks/ReactFeatureFlags.native-fb.js | 1 - packages/shared/forks/ReactFeatureFlags.native-oss.js | 1 - .../shared/forks/ReactFeatureFlags.test-renderer.js | 1 - .../forks/ReactFeatureFlags.test-renderer.native-fb.js | 1 - .../forks/ReactFeatureFlags.test-renderer.www.js | 1 - packages/shared/forks/ReactFeatureFlags.www.js | 1 - 9 files changed, 2 insertions(+), 20 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 1cf98193abcc7..afa0a9c218e53 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -63,7 +63,6 @@ import { } from './ReactNativeFiberInspector'; import { - enableFabricCompleteRootInCommitPhase, passChildrenWhenCloningPersistedNodes, enableLazyPublicInstanceInFabric, } from 'shared/ReactFeatureFlags'; @@ -543,19 +542,14 @@ export function finalizeContainerChildren( container: Container, newChildren: ChildSet, ): void { - if (!enableFabricCompleteRootInCommitPhase) { - completeRoot(container.containerTag, newChildren); - } + // Noop - children will be replaced in replaceContainerChildren } export function replaceContainerChildren( container: Container, newChildren: ChildSet, ): void { - // Noop - children will be replaced in finalizeContainerChildren - if (enableFabricCompleteRootInCommitPhase) { - completeRoot(container.containerTag, newChildren); - } + completeRoot(container.containerTag, newChildren); } export {getClosestInstanceFromNode as getInstanceFromNode}; diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 10bd08970f8e8..5237e312318e5 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -100,11 +100,6 @@ export const enableSuspenseyImages = false; export const enableSrcObject = __EXPERIMENTAL__; -/** - * Switches the Fabric API from doing layout in commit work instead of complete work. - */ -export const enableFabricCompleteRootInCommitPhase = false; - /** * Switches Fiber creation to a simple object instead of a constructor. */ diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index f1ced67c446d9..c2b56602b39f7 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -23,7 +23,6 @@ export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__; export const enablePersistedModeClonedFlag = __VARIANT__; export const enableShallowPropDiffing = __VARIANT__; export const passChildrenWhenCloningPersistedNodes = __VARIANT__; -export const enableFabricCompleteRootInCommitPhase = __VARIANT__; export const enableSiblingPrerendering = __VARIANT__; export const enableFastAddPropertiesInDiffing = __VARIANT__; export const enableLazyPublicInstanceInFabric = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 6bc3f7b1d1e1f..93c3ebff00886 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -20,7 +20,6 @@ const dynamicFlags: DynamicExportsType = (dynamicFlagsUntyped: any); // the exports object every time a flag is read. export const { alwaysThrottleRetries, - enableFabricCompleteRootInCommitPhase, enableHiddenSubtreeInsertionEffectCleanup, enableObjectFiber, enablePersistedModeClonedFlag, diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index cb2d97eb70c92..c1e7cebdaecba 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -30,7 +30,6 @@ export const enableAsyncIterableChildren = false; export const enableCPUSuspense = false; export const enableCreateEventHandleAPI = false; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; -export const enableFabricCompleteRootInCommitPhase = false; export const enableMoveBefore = true; export const enableFizzExternalRuntime = true; export const enableHalt = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index e4295dec55500..21a95d51cdc9e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -36,7 +36,6 @@ export const enableUseEffectEventHook = false; export const favorSafetyOverHydrationPerf = true; export const enableLegacyFBSupport = false; export const enableMoveBefore = false; -export const enableFabricCompleteRootInCommitPhase = false; export const enableHiddenSubtreeInsertionEffectCleanup = false; export const enableHydrationLaneScheduling = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index dcded1c0a61b4..478e1d7953c39 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -60,7 +60,6 @@ export const renameElementSymbol = false; export const retryLaneExpirationMs = 5000; export const syncLaneExpirationMs = 250; export const transitionLaneExpirationMs = 5000; -export const enableFabricCompleteRootInCommitPhase = false; export const enableSiblingPrerendering = true; export const enableHydrationLaneScheduling = true; export const enableYieldingBeforePassive = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 71d6f09abb8c4..247950f1fb37b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -39,7 +39,6 @@ export const favorSafetyOverHydrationPerf = true; export const enableLegacyFBSupport = false; export const enableMoveBefore = false; export const enableRenderableContext = false; -export const enableFabricCompleteRootInCommitPhase = false; export const enableHiddenSubtreeInsertionEffectCleanup = true; export const enableRetryLaneExpiration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 079b6e8e92edc..78ff747a065c5 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -48,7 +48,6 @@ export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; export const enableUpdaterTracking = __PROFILE__; -export const enableFabricCompleteRootInCommitPhase = false; export const enableSuspenseAvoidThisFallback = true;