From 526dd340b3e77193846fe5eed02b9bb89d7c2d15 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 2 Jun 2025 11:43:45 -0400 Subject: [PATCH 1/2] [compiler][patch] Emit unary expressions instead of negative numbers (#33383) This is a babel bug + edge case. Babel compact mode produces invalid JavaScript (i.e. parse error) when given a `NumericLiteral` with a negative value. See https://codesandbox.io/p/devbox/5d47fr for repro. --- .../ReactiveScopes/CodegenReactiveFunction.ts | 13 ++++- ...el-repro-compact-negative-number.expect.md | 56 +++++++++++++++++++ .../babel-repro-compact-negative-number.js | 15 +++++ compiler/packages/snap/src/compiler.ts | 1 + 4 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/babel-repro-compact-negative-number.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/babel-repro-compact-negative-number.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 33a124dcec6e2..17c62c02a6ee8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1726,7 +1726,7 @@ function codegenInstructionValue( } case 'UnaryExpression': { value = t.unaryExpression( - instrValue.operator as 'throw', // todo + instrValue.operator, codegenPlaceToExpression(cx, instrValue.value), ); break; @@ -2582,7 +2582,16 @@ function codegenValue( value: boolean | number | string | null | undefined, ): t.Expression { if (typeof value === 'number') { - return t.numericLiteral(value); + if (value < 0) { + /** + * Babel's code generator produces invalid JS for negative numbers when + * run with { compact: true }. + * See repro https://codesandbox.io/p/devbox/5d47fr + */ + return t.unaryExpression('-', t.numericLiteral(-value), false); + } else { + return t.numericLiteral(value); + } } else if (typeof value === 'boolean') { return t.booleanLiteral(value); } else if (typeof value === 'string') { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/babel-repro-compact-negative-number.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/babel-repro-compact-negative-number.expect.md new file mode 100644 index 0000000000000..70e19e0744a4a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/babel-repro-compact-negative-number.expect.md @@ -0,0 +1,56 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +function Repro(props) { + const MY_CONST = -2; + return {props.arg - MY_CONST}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Repro, + params: [ + { + arg: 3, + }, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +function Repro(props) { + const $ = _c(2); + + const t0 = props.arg - -2; + let t1; + if ($[0] !== t0) { + t1 = {t0}; + $[0] = t0; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Repro, + params: [ + { + arg: 3, + }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"children":5}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/babel-repro-compact-negative-number.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/babel-repro-compact-negative-number.js new file mode 100644 index 0000000000000..891589bc981d4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/babel-repro-compact-negative-number.js @@ -0,0 +1,15 @@ +import {Stringify} from 'shared-runtime'; + +function Repro(props) { + const MY_CONST = -2; + return {props.arg - MY_CONST}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Repro, + params: [ + { + arg: 3, + }, + ], +}; diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 0cadf30bf0eb1..7241ed51492bc 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -242,6 +242,7 @@ export async function transformFixtureInput( filename: virtualFilepath, highlightCode: false, retainLines: true, + compact: true, plugins: [ [plugin, options], 'babel-plugin-fbt', From 4a1f29079ccc61659e026bbcf205bc8d53780927 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Mon, 2 Jun 2025 19:27:49 +0200 Subject: [PATCH 2/2] [Fizz] Add Owner Stacks when render is aborted (#32735) --- .../src/ReactNoopServer.js | 1 + packages/react-server/src/ReactFizzServer.js | 27 +++++++++- .../src/__tests__/ReactServer-test.js | 54 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 0244e3dfab5fd..59f0fafa5d21f 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -364,6 +364,7 @@ function render(children: React$Element, options?: Options): Destination { children, null, null, + null, options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, options ? options.onAllReady : undefined, diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 35c7909396edb..a18d8d16748f9 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -4762,6 +4762,27 @@ function abortTask(task: Task, request: Request, error: mixed): void { } } +function abortTaskDEV(task: Task, request: Request, error: mixed): void { + if (__DEV__) { + const prevTaskInDEV = currentTaskInDEV; + const prevGetCurrentStackImpl = ReactSharedInternals.getCurrentStack; + setCurrentTaskInDEV(task); + ReactSharedInternals.getCurrentStack = getCurrentStackInDEV; + try { + abortTask(task, request, error); + } finally { + setCurrentTaskInDEV(prevTaskInDEV); + ReactSharedInternals.getCurrentStack = prevGetCurrentStackImpl; + } + } else { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'abortTaskDEV should never be called in production mode. This is a bug in React.', + ); + } +} + function safelyEmitEarlyPreloads( request: Request, shellComplete: boolean, @@ -6111,7 +6132,11 @@ export function abort(request: Request, reason: mixed): void { // This error isn't necessarily fatal in this case but we need to stash it // so we can use it to abort any pending work request.fatalError = error; - abortableTasks.forEach(task => abortTask(task, request, error)); + if (__DEV__) { + abortableTasks.forEach(task => abortTaskDEV(task, request, error)); + } else { + abortableTasks.forEach(task => abortTask(task, request, error)); + } abortableTasks.clear(); } if (request.destination !== null) { diff --git a/packages/react-server/src/__tests__/ReactServer-test.js b/packages/react-server/src/__tests__/ReactServer-test.js index d827e82de0cac..c6504fd21f578 100644 --- a/packages/react-server/src/__tests__/ReactServer-test.js +++ b/packages/react-server/src/__tests__/ReactServer-test.js @@ -10,13 +10,28 @@ 'use strict'; +let act; let React; let ReactNoopServer; +function normalizeCodeLocInfo(str) { + return ( + str && + str.replace(/^ +(?:at|in) ([\S]+)[^\n]*/gm, function (m, name) { + const dot = name.lastIndexOf('.'); + if (dot !== -1) { + name = name.slice(dot + 1); + } + return ' in ' + name + (/\d/.test(m) ? ' (at **)' : ''); + }) + ); +} + describe('ReactServer', () => { beforeEach(() => { jest.resetModules(); + act = require('internal-test-utils').act; React = require('react'); ReactNoopServer = require('react-noop-renderer/server'); }); @@ -32,4 +47,43 @@ describe('ReactServer', () => { const result = ReactNoopServer.render(
hello world
); expect(result.root).toEqual(div('hello world')); }); + + it('has Owner Stacks in DEV when aborted', async () => { + function Component({promise}) { + React.use(promise); + return
Hello, Dave!
; + } + function App({promise}) { + return ; + } + + let caughtError; + let componentStack; + let ownerStack; + const result = ReactNoopServer.render( + {})} />, + { + onError: (error, errorInfo) => { + caughtError = error; + componentStack = errorInfo.componentStack; + ownerStack = __DEV__ ? React.captureOwnerStack() : null; + }, + }, + ); + + await act(async () => { + result.abort(); + }); + expect(caughtError).toEqual( + expect.objectContaining({ + message: 'The render was aborted by the server without a reason.', + }), + ); + expect(normalizeCodeLocInfo(componentStack)).toEqual( + '\n in Component (at **)' + '\n in App (at **)', + ); + expect(normalizeCodeLocInfo(ownerStack)).toEqual( + __DEV__ ? '\n in App (at **)' : null, + ); + }); });