From ea6e05912aa43a0bbfbee381752caa1817a41a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sun, 26 May 2024 17:55:57 -0400 Subject: [PATCH 01/26] [Fiber] Enable Native console.createTask Stacks When Available (#29223) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #29206 and #29221. This disables appending owner stacks to console when `console.createTask` is available in the environment. Instead we rely on native "async" stacks that end up looking like this with source maps and ignore list enabled. Screenshot 2024-05-22 at 4 00 27 PM Unfortunately Chrome requires a string name for each async stack and, worse, a suffix of `(async)` is automatically added which is very confusing since it seems like it might be an async component or something which it is not. In this case it's not so bad because it's nice to refer to the host component which otherwise doesn't have a stack frame since it's internal. However, if there were more owners here there would also be a ` (async)` which ends up being kind of duplicative. If the Chrome DevTools is not open from the start of the app, then `console.createTask` is disabled and so you lose the stack for those errors (or those parents if the devtools is opened later). Unlike our appended ones that are always added. That's unfortunate and likely to be a bit of a DX issue but it's also nice that it saves on perf in DEV mode for those cases. Framework dialogs can still surface the stack since we also track it in user space in parallel. This currently doesn't track Server Components yet. We need a more clever hack for that part in a follow up. I think I probably need to also add something to React DevTools to disable its stacks for this case too. Since it looks for stacks in the console.error and adds a stack otherwise. Since we don't add them anymore from the runtime, the DevTools adds them instead. --- .../src/backend/DevToolsFiberComponentStack.js | 7 +++++++ packages/react-devtools-shared/src/backend/console.js | 10 ++++++++-- packages/react-reconciler/src/ReactCurrentFiber.js | 7 +++++++ packages/shared/consoleWithStackDev.js | 9 ++++++++- 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js b/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js index 6d99cf4f21..a4311797de 100644 --- a/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js +++ b/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js @@ -98,3 +98,10 @@ export function getStackByFiberInDevAndProd( return '\nError generating stack: ' + x.message + '\n' + x.stack; } } + +export function supportsNativeConsoleTasks(fiber: Fiber): boolean { + // If this Fiber supports native console.createTask then we are already running + // inside a native async stack trace if it's active - meaning the DevTools is open. + // Ideally we'd detect if this task was created while the DevTools was open or not. + return !!fiber._debugTask; +} diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 40649abe9e..13c2249013 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -18,7 +18,10 @@ import type { import {format, formatWithStyles} from './utils'; import {getInternalReactConstants, getDispatcherRef} from './renderer'; -import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack'; +import { + getStackByFiberInDevAndProd, + supportsNativeConsoleTasks, +} from './DevToolsFiberComponentStack'; import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags'; import {castBool, castBrowserTheme} from '../utils'; @@ -235,7 +238,10 @@ export function patch({ } } - if (shouldAppendWarningStack) { + if ( + shouldAppendWarningStack && + !supportsNativeConsoleTasks(current) + ) { const componentStack = getStackByFiberInDevAndProd( workTagMap, current, diff --git a/packages/react-reconciler/src/ReactCurrentFiber.js b/packages/react-reconciler/src/ReactCurrentFiber.js index 98e82247d5..cf0c2543a5 100644 --- a/packages/react-reconciler/src/ReactCurrentFiber.js +++ b/packages/react-reconciler/src/ReactCurrentFiber.js @@ -74,6 +74,13 @@ export function runWithFiberInDEV( const previousFiber = current; setCurrentFiber(fiber); try { + if (enableOwnerStacks) { + if (fiber !== null && fiber._debugTask) { + return fiber._debugTask.run( + callback.bind(null, arg0, arg1, arg2, arg3, arg4), + ); + } + } return callback(arg0, arg1, arg2, arg3, arg4); } finally { current = previousFiber; diff --git a/packages/shared/consoleWithStackDev.js b/packages/shared/consoleWithStackDev.js index 464f1b0999..bdcf754802 100644 --- a/packages/shared/consoleWithStackDev.js +++ b/packages/shared/consoleWithStackDev.js @@ -6,6 +6,7 @@ */ import ReactSharedInternals from 'shared/ReactSharedInternals'; +import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; let suppressWarning = false; export function setSuppressWarning(newSuppressWarning) { @@ -36,6 +37,9 @@ export function error(format, ...args) { } } +// eslint-disable-next-line react-internal/no-production-logging +const supportsCreateTask = __DEV__ && enableOwnerStacks && !!console.createTask; + function printWarning(level, format, args) { // When changing this logic, you might want to also // update consoleWithStackDev.www.js as well. @@ -43,7 +47,10 @@ function printWarning(level, format, args) { const isErrorLogger = format === '%s\n\n%s\n' || format === '%o\n\n%s\n\n%s\n'; - if (ReactSharedInternals.getCurrentStack) { + if (!supportsCreateTask && ReactSharedInternals.getCurrentStack) { + // We only add the current stack to the console when createTask is not supported. + // Since createTask requires DevTools to be open to work, this means that stacks + // can be lost while DevTools isn't open but we can't detect this. const stack = ReactSharedInternals.getCurrentStack(); if (stack !== '') { format += '%s'; From 6f23540c7d39d7da2091284322008dadd055c031 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Tue, 28 May 2024 11:07:31 +0100 Subject: [PATCH 02/26] cleanup[react-devtools]: remove unused supportsProfiling flag from store config (#29193) Looks like this is unused --- .../src/main/getProfilingFlags.js | 4 +--- .../react-devtools-extensions/src/main/index.js | 3 +-- packages/react-devtools-fusebox/src/frontend.js | 7 +------ packages/react-devtools-inline/src/frontend.js | 7 +------ .../react-devtools-shared/src/devtools/store.js | 14 +------------- 5 files changed, 5 insertions(+), 30 deletions(-) diff --git a/packages/react-devtools-extensions/src/main/getProfilingFlags.js b/packages/react-devtools-extensions/src/main/getProfilingFlags.js index 760b0e4835..4fcc7a8ea3 100644 --- a/packages/react-devtools-extensions/src/main/getProfilingFlags.js +++ b/packages/react-devtools-extensions/src/main/getProfilingFlags.js @@ -9,15 +9,13 @@ function getProfilingFlags() { // This avoids flashing a temporary "Profiling not supported" message in the Profiler tab, // after a user has clicked the "reload and profile" button. let isProfiling = false; - let supportsProfiling = false; if (localStorageGetItem(LOCAL_STORAGE_SUPPORTS_PROFILING_KEY) === 'true') { - supportsProfiling = true; isProfiling = true; localStorageRemoveItem(LOCAL_STORAGE_SUPPORTS_PROFILING_KEY); } - return {isProfiling, supportsProfiling}; + return {isProfiling}; } export default getProfilingFlags; diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js index de817e268f..224e4cd4b4 100644 --- a/packages/react-devtools-extensions/src/main/index.js +++ b/packages/react-devtools-extensions/src/main/index.js @@ -89,12 +89,11 @@ function createBridge() { function createBridgeAndStore() { createBridge(); - const {isProfiling, supportsProfiling} = getProfilingFlags(); + const {isProfiling} = getProfilingFlags(); store = new Store(bridge, { isProfiling, supportsReloadAndProfile: __IS_CHROME__ || __IS_EDGE__, - supportsProfiling, // At this time, the timeline can only parse Chrome performance profiles. supportsTimeline: __IS_CHROME__, supportsTraceUpdates: true, diff --git a/packages/react-devtools-fusebox/src/frontend.js b/packages/react-devtools-fusebox/src/frontend.js index d8a019b36a..ca236031dd 100644 --- a/packages/react-devtools-fusebox/src/frontend.js +++ b/packages/react-devtools-fusebox/src/frontend.js @@ -23,12 +23,7 @@ import type { ViewElementSource, CanViewElementSource, } from 'react-devtools-shared/src/devtools/views/DevTools'; - -type Config = { - checkBridgeProtocolCompatibility?: boolean, - supportsNativeInspection?: boolean, - supportsProfiling?: boolean, -}; +import type {Config} from 'react-devtools-shared/src/devtools/store'; export function createBridge(wall?: Wall): FrontendBridge { if (wall != null) { diff --git a/packages/react-devtools-inline/src/frontend.js b/packages/react-devtools-inline/src/frontend.js index b1a81e0400..d0e0fbfccc 100644 --- a/packages/react-devtools-inline/src/frontend.js +++ b/packages/react-devtools-inline/src/frontend.js @@ -16,12 +16,7 @@ import { import type {Wall} from 'react-devtools-shared/src/frontend/types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type {Props} from 'react-devtools-shared/src/devtools/views/DevTools'; - -type Config = { - checkBridgeProtocolCompatibility?: boolean, - supportsNativeInspection?: boolean, - supportsProfiling?: boolean, -}; +import type {Config} from 'react-devtools-shared/src/devtools/store'; export function createStore(bridge: FrontendBridge, config?: Config): Store { return new Store(bridge, { diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 1d0632072d..3eb589b903 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -68,11 +68,10 @@ const LOCAL_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY = type ErrorAndWarningTuples = Array<{id: number, index: number}>; -type Config = { +export type Config = { checkBridgeProtocolCompatibility?: boolean, isProfiling?: boolean, supportsNativeInspection?: boolean, - supportsProfiling?: boolean, supportsReloadAndProfile?: boolean, supportsTimeline?: boolean, supportsTraceUpdates?: boolean, @@ -174,7 +173,6 @@ export default class Store extends EventEmitter<{ // These options may be initially set by a configuration option when constructing the Store. _supportsNativeInspection: boolean = true; - _supportsProfiling: boolean = false; _supportsReloadAndProfile: boolean = false; _supportsTimeline: boolean = false; _supportsTraceUpdates: boolean = false; @@ -214,15 +212,11 @@ export default class Store extends EventEmitter<{ const { supportsNativeInspection, - supportsProfiling, supportsReloadAndProfile, supportsTimeline, supportsTraceUpdates, } = config; this._supportsNativeInspection = supportsNativeInspection !== false; - if (supportsProfiling) { - this._supportsProfiling = true; - } if (supportsReloadAndProfile) { this._supportsReloadAndProfile = true; } @@ -449,12 +443,6 @@ export default class Store extends EventEmitter<{ return this._isNativeStyleEditorSupported; } - // This build of DevTools supports the legacy profiler. - // This is a static flag, controlled by the Store config. - get supportsProfiling(): boolean { - return this._supportsProfiling; - } - get supportsReloadAndProfile(): boolean { // Does the DevTools shell support reloading and eagerly injecting the renderer interface? // And if so, can the backend use the localStorage API and sync XHR? From 4ec6a6f71475a6f2fee39a0e604ddbbd2f124164 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Tue, 28 May 2024 10:06:05 -0700 Subject: [PATCH 03/26] Repro function expr hoisting (#29615) Modified version of @mofeiZ's #29232 with CI passing (had to run prettier) --------- Co-authored-by: Mofei Zhang --- ...ug-invalid-hoisting-functionexpr.expect.md | 93 ++++++++++++++ .../bug-invalid-hoisting-functionexpr.tsx | 30 +++++ ...invalid-pruned-scope-leaks-value.expect.md | 119 ++++++++++++++++++ .../bug-invalid-pruned-scope-leaks-value.ts | 43 +++++++ .../packages/snap/src/SproutTodoFilter.ts | 2 + .../snap/src/sprout/shared-runtime.ts | 4 + 6 files changed, 291 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.expect.md new file mode 100644 index 0000000000..37cd740908 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.expect.md @@ -0,0 +1,93 @@ + +## Input + +```javascript +import { Stringify } from "shared-runtime"; + +/** + * We currently hoist the accessed properties of function expressions, + * regardless of control flow. This is simply because we wrote support for + * function expressions before doing a lot of work in PropagateScopeDeps + * to handle conditionally accessed dependencies. + * + * Current evaluator error: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok)
{"shouldInvokeFns":true,"callback":{"kind":"Function","result":null}}
+ * Forget: + * (kind: exception) Cannot read properties of null (reading 'prop') + */ +function Component({ obj, isObjNull }) { + const callback = () => { + if (!isObjNull) { + return obj.prop; + } else { + return null; + } + }; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ obj: null, isObjNull: true }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * We currently hoist the accessed properties of function expressions, + * regardless of control flow. This is simply because we wrote support for + * function expressions before doing a lot of work in PropagateScopeDeps + * to handle conditionally accessed dependencies. + * + * Current evaluator error: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok)
{"shouldInvokeFns":true,"callback":{"kind":"Function","result":null}}
+ * Forget: + * (kind: exception) Cannot read properties of null (reading 'prop') + */ +function Component(t0) { + const $ = _c(5); + const { obj, isObjNull } = t0; + let t1; + if ($[0] !== isObjNull || $[1] !== obj.prop) { + t1 = () => { + if (!isObjNull) { + return obj.prop; + } else { + return null; + } + }; + $[0] = isObjNull; + $[1] = obj.prop; + $[2] = t1; + } else { + t1 = $[2]; + } + const callback = t1; + let t2; + if ($[3] !== callback) { + t2 = ; + $[3] = callback; + $[4] = t2; + } else { + t2 = $[4]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ obj: null, isObjNull: true }], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.tsx new file mode 100644 index 0000000000..047d178868 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.tsx @@ -0,0 +1,30 @@ +import { Stringify } from "shared-runtime"; + +/** + * We currently hoist the accessed properties of function expressions, + * regardless of control flow. This is simply because we wrote support for + * function expressions before doing a lot of work in PropagateScopeDeps + * to handle conditionally accessed dependencies. + * + * Current evaluator error: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok)
{"shouldInvokeFns":true,"callback":{"kind":"Function","result":null}}
+ * Forget: + * (kind: exception) Cannot read properties of null (reading 'prop') + */ +function Component({ obj, isObjNull }) { + const callback = () => { + if (!isObjNull) { + return obj.prop; + } else { + return null; + } + }; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ obj: null, isObjNull: true }], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md new file mode 100644 index 0000000000..d490c6e8f3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md @@ -0,0 +1,119 @@ + +## Input + +```javascript +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening + * those scopes. Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive and does not need to be a memo + * dependency. + * + * Current evaluator error: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * Forget: + * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [[ (exception in render) Invariant Violation: oh no! ]] + * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + */ + +function MyApp({ count }) { + const z = makeObject_Primitives(); + const x = useIdentity(2); + const y = sum(x, count); + mutate(z); + const thing = [y, z]; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening + * those scopes. Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive and does not need to be a memo + * dependency. + * + * Current evaluator error: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * Forget: + * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [[ (exception in render) Invariant Violation: oh no! ]] + * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + */ + +function MyApp(t0) { + const $ = _c(5); + const { count } = t0; + const z = makeObject_Primitives(); + const x = useIdentity(2); + let t1; + if ($[0] !== x || $[1] !== count) { + t1 = sum(x, count); + $[0] = x; + $[1] = count; + $[2] = t1; + } else { + t1 = $[2]; + } + const y = t1; + mutate(z); + let t2; + if ($[3] !== y) { + t2 = [y, z]; + $[3] = y; + $[4] = t2; + } else { + t2 = $[4]; + } + const thing = t2; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts new file mode 100644 index 0000000000..5b9c2527ae --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts @@ -0,0 +1,43 @@ +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening + * those scopes. Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive and does not need to be a memo + * dependency. + * + * Current evaluator error: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * Forget: + * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + * [[ (exception in render) Invariant Violation: oh no! ]] + * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] + */ + +function MyApp({ count }) { + const z = makeObject_Primitives(); + const x = useIdentity(2); + const y = sum(x, count); + mutate(z); + const thing = [y, z]; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 25ac01cdd5..a8fc53606d 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -486,6 +486,8 @@ const skipFilter = new Set([ // bugs "bug-invalid-reactivity-value-block", + "bug-invalid-pruned-scope-leaks-value", + "bug-invalid-hoisting-functionexpr", "original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block", "original-reactive-scopes-fork/bug-hoisted-declaration-with-scope", diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index 0f1dedd152..94fba22c04 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -176,6 +176,10 @@ export function useNoAlias(...args: Array): object { return noAliasObject; } +export function useIdentity(arg: T): T { + return arg; +} + export function invoke, ReturnType>( fn: (...input: T) => ReturnType, ...params: T From 681a4aa81022d4053f990d905d6453c73d2ee644 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 28 May 2024 14:06:30 -0400 Subject: [PATCH 04/26] Throw if React and React DOM versions don't match (#29236) Throw an error during module initialization if the version of the "react-dom" package does not match the version of "react". We used to be more relaxed about this, because the "react" package changed so infrequently. However, we now have many more features that rely on an internal protocol between the two packages, including Hooks, Float, and the compiler runtime. So it's important that both packages are versioned in lockstep. Before this change, a version mismatch would often result in a cryptic internal error with no indication of the root cause. Instead, we will now compare the versions during module initialization and immediately throw an error to catch mistakes as early as possible and provide a clear error message. --- .../react-dom/src/client/ReactDOMClient.js | 3 + .../react-dom/src/client/ReactDOMClientFB.js | 3 + .../src/server/ReactDOMFizzServerBrowser.js | 3 + .../src/server/ReactDOMFizzServerBun.js | 3 + .../src/server/ReactDOMFizzServerEdge.js | 3 + .../src/server/ReactDOMFizzServerNode.js | 3 + .../src/server/ReactDOMFizzStaticBrowser.js | 3 + .../src/server/ReactDOMFizzStaticEdge.js | 3 + .../src/server/ReactDOMFizzStaticNode.js | 3 + .../ensureCorrectIsomorphicReactVersion.js | 24 +++ .../src/ReactNativeRenderer.js | 14 ++ .../__tests__/ReactMismatchedVersions-test.js | 143 ++++++++++++++++++ .../useSyncExternalStoreShared-test.js | 7 + scripts/error-codes/codes.json | 3 +- 14 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 packages/react-dom/src/shared/ensureCorrectIsomorphicReactVersion.js create mode 100644 packages/react/src/__tests__/ReactMismatchedVersions-test.js diff --git a/packages/react-dom/src/client/ReactDOMClient.js b/packages/react-dom/src/client/ReactDOMClient.js index 036fd846a4..3837f322a5 100644 --- a/packages/react-dom/src/client/ReactDOMClient.js +++ b/packages/react-dom/src/client/ReactDOMClient.js @@ -19,6 +19,9 @@ import ReactVersion from 'shared/ReactVersion'; import {getClosestInstanceFromNode} from 'react-dom-bindings/src/client/ReactDOMComponentTree'; import Internals from 'shared/ReactDOMSharedInternals'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + if (__DEV__) { if ( typeof Map !== 'function' || diff --git a/packages/react-dom/src/client/ReactDOMClientFB.js b/packages/react-dom/src/client/ReactDOMClientFB.js index 1f918e3cf6..a398586072 100644 --- a/packages/react-dom/src/client/ReactDOMClientFB.js +++ b/packages/react-dom/src/client/ReactDOMClientFB.js @@ -25,6 +25,9 @@ import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal import {canUseDOM} from 'shared/ExecutionEnvironment'; import ReactVersion from 'shared/ReactVersion'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + import { getClosestInstanceFromNode, getInstanceFromNode, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index 14c4a59792..8879a511d3 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -37,6 +37,9 @@ import { createRootFormatContext, } from 'react-dom-bindings/src/server/ReactFizzConfigDOM'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + type Options = { identifierPrefix?: string, namespaceURI?: string, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBun.js b/packages/react-dom/src/server/ReactDOMFizzServerBun.js index 4cceb66e7c..750c3133c4 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBun.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBun.js @@ -31,6 +31,9 @@ import { createRootFormatContext, } from 'react-dom-bindings/src/server/ReactFizzConfigDOM'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + type Options = { identifierPrefix?: string, namespaceURI?: string, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerEdge.js b/packages/react-dom/src/server/ReactDOMFizzServerEdge.js index 14c4a59792..8879a511d3 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerEdge.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerEdge.js @@ -37,6 +37,9 @@ import { createRootFormatContext, } from 'react-dom-bindings/src/server/ReactFizzConfigDOM'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + type Options = { identifierPrefix?: string, namespaceURI?: string, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index 049849a664..f0c9d75fc7 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -41,6 +41,9 @@ import { createRootFormatContext, } from 'react-dom-bindings/src/server/ReactFizzConfigDOM'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + function createDrainHandler(destination: Destination, request: Request) { return () => startFlowing(request, destination); } diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js b/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js index cbc5cd4044..f5d6a45a18 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js @@ -36,6 +36,9 @@ import { createRootFormatContext, } from 'react-dom-bindings/src/server/ReactFizzConfigDOM'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + type Options = { identifierPrefix?: string, namespaceURI?: string, diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js b/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js index e1fc514b7d..1a2eb1e599 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js @@ -36,6 +36,9 @@ import { createRootFormatContext, } from 'react-dom-bindings/src/server/ReactFizzConfigDOM'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + type Options = { identifierPrefix?: string, namespaceURI?: string, diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticNode.js b/packages/react-dom/src/server/ReactDOMFizzStaticNode.js index 3c3c4116a5..fc25aa75c1 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticNode.js @@ -37,6 +37,9 @@ import { createRootFormatContext, } from 'react-dom-bindings/src/server/ReactFizzConfigDOM'; +import {ensureCorrectIsomorphicReactVersion} from '../shared/ensureCorrectIsomorphicReactVersion'; +ensureCorrectIsomorphicReactVersion(); + type Options = { identifierPrefix?: string, namespaceURI?: string, diff --git a/packages/react-dom/src/shared/ensureCorrectIsomorphicReactVersion.js b/packages/react-dom/src/shared/ensureCorrectIsomorphicReactVersion.js new file mode 100644 index 0000000000..cbfece8983 --- /dev/null +++ b/packages/react-dom/src/shared/ensureCorrectIsomorphicReactVersion.js @@ -0,0 +1,24 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import reactDOMPackageVersion from 'shared/ReactVersion'; +import * as IsomorphicReactPackage from 'react'; + +export function ensureCorrectIsomorphicReactVersion() { + const isomorphicReactPackageVersion = IsomorphicReactPackage.version; + if (isomorphicReactPackageVersion !== reactDOMPackageVersion) { + throw new Error( + 'Incompatible React versions: The "react" and "react-dom" packages must ' + + 'have the exact same version. Instead got:\n' + + ` - react: ${isomorphicReactPackageVersion}\n` + + ` - react-dom: ${reactDOMPackageVersion}\n` + + 'Learn more: https://react.dev/warnings/version-mismatch', + ); + } +} diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index 528c8abe91..5c2957aed1 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -56,6 +56,20 @@ import {disableLegacyMode} from 'shared/ReactFeatureFlags'; // Module provided by RN: import {ReactFiberErrorDialog} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'; +import reactNativePackageVersion from 'shared/ReactVersion'; +import * as IsomorphicReactPackage from 'react'; + +const isomorphicReactPackageVersion = IsomorphicReactPackage.version; +if (isomorphicReactPackageVersion !== reactNativePackageVersion) { + throw new Error( + 'Incompatible React versions: The "react" and "react-native-renderer" packages must ' + + 'have the exact same version. Instead got:\n' + + ` - react: ${isomorphicReactPackageVersion}\n` + + ` - react-native-renderer: ${reactNativePackageVersion}\n` + + 'Learn more: https://react.dev/warnings/version-mismatch', + ); +} + if (typeof ReactFiberErrorDialog.showErrorDialog !== 'function') { throw new Error( 'Expected ReactFiberErrorDialog.showErrorDialog to be a function.', diff --git a/packages/react/src/__tests__/ReactMismatchedVersions-test.js b/packages/react/src/__tests__/ReactMismatchedVersions-test.js new file mode 100644 index 0000000000..cee86e5087 --- /dev/null +++ b/packages/react/src/__tests__/ReactMismatchedVersions-test.js @@ -0,0 +1,143 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +describe('ReactMismatchedVersions-test', () => { + // Polyfills for test environment + global.ReadableStream = + require('web-streams-polyfill/ponyfill/es6').ReadableStream; + global.TextEncoder = require('util').TextEncoder; + + let React; + let actualReactVersion; + + beforeEach(() => { + jest.resetModules(); + jest.mock('react', () => { + const actualReact = jest.requireActual('react'); + return { + ...actualReact, + version: '18.0.0-whoa-this-aint-the-right-react', + __actualVersion: actualReact.version, + }; + }); + React = require('react'); + actualReactVersion = React.__actualVersion; + }); + + test('importing "react-dom/client" throws if version does not match React version', async () => { + expect(() => require('react-dom/client')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + // When running in source mode, we lazily require the implementation to + // simulate the static config dependency injection we do at build time. So it + // only errors once you call something and trigger the require. Running the + // test in build mode is sufficient. + // @gate !source + test('importing "react-dom/server" throws if version does not match React version', async () => { + expect(() => require('react-dom/server')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + // @gate !source + test('importing "react-dom/server.node" throws if version does not match React version', async () => { + expect(() => require('react-dom/server.node')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + // @gate !source + test('importing "react-dom/server.browser" throws if version does not match React version', async () => { + expect(() => require('react-dom/server.browser')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + // @gate !source + test('importing "react-dom/server.bun" throws if version does not match React version', async () => { + expect(() => require('react-dom/server.bun')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + // @gate !source + test('importing "react-dom/server.edge" throws if version does not match React version', async () => { + expect(() => require('react-dom/server.edge')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + test('importing "react-dom/static" throws if version does not match React version', async () => { + expect(() => require('react-dom/static')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + test('importing "react-dom/static.node" throws if version does not match React version', async () => { + expect(() => require('react-dom/static.node')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + test('importing "react-dom/static.browser" throws if version does not match React version', async () => { + expect(() => require('react-dom/static.browser')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + test('importing "react-dom/static.edge" throws if version does not match React version', async () => { + expect(() => require('react-dom/static.edge')).toThrow( + 'Incompatible React versions: The "react" and "react-dom" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-dom: ${actualReactVersion}`, + ); + }); + + // @gate source + test('importing "react-native-renderer" throws if version does not match React version', async () => { + expect(() => require('react-native-renderer')).toThrow( + 'Incompatible React versions: The "react" and "react-native-renderer" packages ' + + 'must have the exact same version. Instead got:\n' + + ' - react: 18.0.0-whoa-this-aint-the-right-react\n' + + ` - react-native-renderer: ${actualReactVersion}`, + ); + }); +}); diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js index 8f8fb8e9a9..1ad3de60da 100644 --- a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js +++ b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js @@ -43,6 +43,13 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { : 'react-dom-17/umd/react-dom.production.min.js', ), ); + jest.mock('react-dom/client', () => + jest.requireActual( + __DEV__ + ? 'react-dom-17/umd/react-dom.development.js' + : 'react-dom-17/umd/react-dom.production.min.js', + ), + ); // Because React 17 prints extra logs we need to ignore them. originalError = console.error; console.error = jest.fn(); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index f0e73bd6fb..c26ae4cce2 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -511,5 +511,6 @@ "523": "The render was aborted due to being postponed.", "524": "Values cannot be passed to next() of AsyncIterables passed to Client Components.", "525": "A React Element from an older version of React was rendered. This is not supported. It can happen if:\n- Multiple copies of the \"react\" package is used.\n- A library pre-bundled an old copy of \"react\" or \"react/jsx-runtime\".\n- A compiler tries to \"inline\" JSX instead of using the runtime.", - "526": "Could not reference an opaque temporary reference. This is likely due to misconfiguring the temporaryReferences options on the server." + "526": "Could not reference an opaque temporary reference. This is likely due to misconfiguring the temporaryReferences options on the server.", + "527": "Incompatible React versions: The \"react\" and \"react-dom\" packages must have the exact same version. Instead got:\n - react: %s\n - react-dom: %s\nLearn more: https://react.dev/warnings/version-mismatch" } From 2787eebe52864356252a280fd811cd9d52807a82 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 28 May 2024 19:55:14 +0100 Subject: [PATCH 05/26] Clean up disableDOMTestUtils (#29610) `disableDOMTestUtils` and the FB build `ReactTestUtilsFB` allowed us to finish migrating internal callsites off of ReactTestUtils. Now that usage is cleaned up, we can remove the flag, build artifact, and test coverage for the deprecated utility methods. --- .../src/__tests__/ReactTestUtils-test.js | 735 --------------- .../src/test-utils/ReactTestUtilsFB.js | 884 ------------------ packages/react-dom/test-utils.fb.js | 10 - packages/shared/ReactFeatureFlags.js | 2 - .../forks/ReactFeatureFlags.native-fb.js | 2 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - ...actFeatureFlags.test-renderer.native-fb.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 2 - scripts/rollup/bundles.js | 2 +- 11 files changed, 1 insertion(+), 1640 deletions(-) delete mode 100644 packages/react-dom/src/__tests__/ReactTestUtils-test.js delete mode 100644 packages/react-dom/src/test-utils/ReactTestUtilsFB.js delete mode 100644 packages/react-dom/test-utils.fb.js diff --git a/packages/react-dom/src/__tests__/ReactTestUtils-test.js b/packages/react-dom/src/__tests__/ReactTestUtils-test.js deleted file mode 100644 index 534b3538eb..0000000000 --- a/packages/react-dom/src/__tests__/ReactTestUtils-test.js +++ /dev/null @@ -1,735 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @emails react-core - */ - -'use strict'; - -let React; -let ReactDOMClient; -let ReactDOMServer; -let ReactTestUtils; -let act; - -function getTestDocument(markup) { - const doc = document.implementation.createHTMLDocument(''); - doc.open(); - doc.write( - markup || - 'test doc', - ); - doc.close(); - return doc; -} - -describe('ReactTestUtils', () => { - beforeEach(() => { - React = require('react'); - ReactDOMClient = require('react-dom/client'); - ReactDOMServer = require('react-dom/server'); - ReactTestUtils = require('react-dom/test-utils'); - act = require('internal-test-utils').act; - }); - - // @gate !disableDOMTestUtils - it('Simulate should have locally attached media events', () => { - expect(Object.keys(ReactTestUtils.Simulate).sort()).toMatchInlineSnapshot(` - [ - "abort", - "animationEnd", - "animationIteration", - "animationStart", - "auxClick", - "beforeInput", - "beforeToggle", - "blur", - "canPlay", - "canPlayThrough", - "cancel", - "change", - "click", - "close", - "compositionEnd", - "compositionStart", - "compositionUpdate", - "contextMenu", - "copy", - "cut", - "doubleClick", - "drag", - "dragEnd", - "dragEnter", - "dragExit", - "dragLeave", - "dragOver", - "dragStart", - "drop", - "durationChange", - "emptied", - "encrypted", - "ended", - "error", - "focus", - "gotPointerCapture", - "input", - "invalid", - "keyDown", - "keyPress", - "keyUp", - "load", - "loadStart", - "loadedData", - "loadedMetadata", - "lostPointerCapture", - "mouseDown", - "mouseEnter", - "mouseLeave", - "mouseMove", - "mouseOut", - "mouseOver", - "mouseUp", - "paste", - "pause", - "play", - "playing", - "pointerCancel", - "pointerDown", - "pointerEnter", - "pointerLeave", - "pointerMove", - "pointerOut", - "pointerOver", - "pointerUp", - "progress", - "rateChange", - "reset", - "resize", - "scroll", - "seeked", - "seeking", - "select", - "stalled", - "submit", - "suspend", - "timeUpdate", - "toggle", - "touchCancel", - "touchEnd", - "touchMove", - "touchStart", - "transitionCancel", - "transitionEnd", - "transitionRun", - "transitionStart", - "volumeChange", - "waiting", - "wheel", - ] - `); - }); - - // @gate !disableDOMTestUtils - it('gives Jest mocks a passthrough implementation with mockComponent()', async () => { - class MockedComponent extends React.Component { - render() { - throw new Error('Should not get here.'); - } - } - // This is close enough to what a Jest mock would give us. - MockedComponent.prototype.render = jest.fn(); - - // Patch it up so it returns its children. - expect(() => ReactTestUtils.mockComponent(MockedComponent)).toWarnDev( - 'ReactTestUtils.mockComponent() is deprecated. ' + - 'Use shallow rendering or jest.mock() instead.\n\n' + - 'See https://react.dev/link/test-utils-mock-component for more information.', - {withoutStack: true}, - ); - - // De-duplication check - ReactTestUtils.mockComponent(MockedComponent); - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(Hello); - }); - - expect(container.textContent).toBe('Hello'); - }); - - // @gate !disableDOMTestUtils - it('can scryRenderedComponentsWithType', async () => { - class Child extends React.Component { - render() { - return null; - } - } - class Wrapper extends React.Component { - render() { - return ( -
- -
- ); - } - } - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let renderedComponent; - await act(() => { - root.render( (renderedComponent = current)} />); - }); - const scryResults = ReactTestUtils.scryRenderedComponentsWithType( - renderedComponent, - Child, - ); - expect(scryResults.length).toBe(1); - }); - - // @gate !disableDOMTestUtils - it('can scryRenderedDOMComponentsWithClass with TextComponent', async () => { - class Wrapper extends React.Component { - render() { - return ( -
- Hello Jim -
- ); - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let renderedComponent; - await act(() => { - root.render( (renderedComponent = current)} />); - }); - const scryResults = ReactTestUtils.scryRenderedDOMComponentsWithClass( - renderedComponent, - 'NonExistentClass', - ); - expect(scryResults.length).toBe(0); - }); - - // @gate !disableDOMTestUtils - it('can scryRenderedDOMComponentsWithClass with className contains \\n', async () => { - class Wrapper extends React.Component { - render() { - return ( -
- Hello Jim -
- ); - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let renderedComponent; - await act(() => { - root.render( (renderedComponent = current)} />); - }); - const scryResults = ReactTestUtils.scryRenderedDOMComponentsWithClass( - renderedComponent, - 'x', - ); - expect(scryResults.length).toBe(1); - }); - - // @gate !disableDOMTestUtils - it('can scryRenderedDOMComponentsWithClass with multiple classes', async () => { - class Wrapper extends React.Component { - render() { - return ( -
- Hello Jim -
- ); - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let renderedComponent; - await act(() => { - root.render( (renderedComponent = current)} />); - }); - const scryResults1 = ReactTestUtils.scryRenderedDOMComponentsWithClass( - renderedComponent, - 'x y', - ); - expect(scryResults1.length).toBe(1); - - const scryResults2 = ReactTestUtils.scryRenderedDOMComponentsWithClass( - renderedComponent, - 'x z', - ); - expect(scryResults2.length).toBe(1); - - const scryResults3 = ReactTestUtils.scryRenderedDOMComponentsWithClass( - renderedComponent, - ['x', 'y'], - ); - expect(scryResults3.length).toBe(1); - - expect(scryResults1[0]).toBe(scryResults2[0]); - expect(scryResults1[0]).toBe(scryResults3[0]); - - const scryResults4 = ReactTestUtils.scryRenderedDOMComponentsWithClass( - renderedComponent, - ['x', 'a'], - ); - expect(scryResults4.length).toBe(0); - - const scryResults5 = ReactTestUtils.scryRenderedDOMComponentsWithClass( - renderedComponent, - ['x a'], - ); - expect(scryResults5.length).toBe(0); - }); - - // @gate !disableDOMTestUtils - it('traverses children in the correct order', async () => { - class Wrapper extends React.Component { - render() { - return
{this.props.children}
; - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - {null} -
purple
- , - ); - }); - let tree; - await act(() => { - root.render( - (tree = current)}> -
orange
-
purple
- , - ); - }); - - const log = []; - ReactTestUtils.findAllInRenderedTree(tree, function (child) { - if (ReactTestUtils.isDOMComponent(child)) { - log.push(child.textContent); - } - }); - - // Should be document order, not mount order (which would be purple, orange) - expect(log).toEqual(['orangepurple', 'orange', 'purple']); - }); - - // @gate !disableDOMTestUtils - it('should support injected wrapper components as DOM components', async () => { - const injectedDOMComponents = [ - 'button', - 'form', - 'iframe', - 'img', - 'input', - 'option', - 'select', - 'textarea', - ]; - - // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const type of injectedDOMComponents) { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let testComponent; - await act(() => { - root.render( - React.createElement(type, { - ref: current => (testComponent = current), - }), - ); - }); - - expect(testComponent.tagName).toBe(type.toUpperCase()); - expect(ReactTestUtils.isDOMComponent(testComponent)).toBe(true); - } - - // Full-page components (html, head, body) can't be rendered into a div - // directly... - class Root extends React.Component { - htmlRef = React.createRef(); - headRef = React.createRef(); - bodyRef = React.createRef(); - - render() { - return ( - - - hello - - hello, world - - ); - } - } - - const markup = ReactDOMServer.renderToString(); - const testDocument = getTestDocument(markup); - let component; - await act(() => { - ReactDOMClient.hydrateRoot( - testDocument, - (component = current)} />, - ); - }); - - expect(component.htmlRef.current.tagName).toBe('HTML'); - expect(component.headRef.current.tagName).toBe('HEAD'); - expect(component.bodyRef.current.tagName).toBe('BODY'); - expect(ReactTestUtils.isDOMComponent(component.htmlRef.current)).toBe(true); - expect(ReactTestUtils.isDOMComponent(component.headRef.current)).toBe(true); - expect(ReactTestUtils.isDOMComponent(component.bodyRef.current)).toBe(true); - }); - - // @gate !disableDOMTestUtils - it('can scry with stateless components involved', async () => { - const Function = () => ( -
-
-
- ); - - class SomeComponent extends React.Component { - render() { - return ( -
- -
-
- ); - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let inst; - await act(() => { - root.render( (inst = current)} />); - }); - - const hrs = ReactTestUtils.scryRenderedDOMComponentsWithTag(inst, 'hr'); - expect(hrs.length).toBe(2); - }); - - // @gate !disableDOMTestUtils - it('provides a clear error when passing invalid objects to scry', () => { - // This is probably too relaxed but it's existing behavior. - ReactTestUtils.findAllInRenderedTree(null, 'span'); - ReactTestUtils.findAllInRenderedTree(undefined, 'span'); - ReactTestUtils.findAllInRenderedTree('', 'span'); - ReactTestUtils.findAllInRenderedTree(0, 'span'); - ReactTestUtils.findAllInRenderedTree(false, 'span'); - - expect(() => { - ReactTestUtils.findAllInRenderedTree([], 'span'); - }).toThrow( - 'The first argument must be a React class instance. ' + - 'Instead received: an array.', - ); - expect(() => { - ReactTestUtils.scryRenderedDOMComponentsWithClass(10, 'button'); - }).toThrow( - 'The first argument must be a React class instance. ' + - 'Instead received: 10.', - ); - expect(() => { - ReactTestUtils.findRenderedDOMComponentWithClass('hello', 'button'); - }).toThrow( - 'The first argument must be a React class instance. ' + - 'Instead received: hello.', - ); - expect(() => { - ReactTestUtils.scryRenderedDOMComponentsWithTag( - {x: true, y: false}, - 'span', - ); - }).toThrow( - 'The first argument must be a React class instance. ' + - 'Instead received: object with keys {x, y}.', - ); - const div = document.createElement('div'); - expect(() => { - ReactTestUtils.findRenderedDOMComponentWithTag(div, 'span'); - }).toThrow( - 'The first argument must be a React class instance. ' + - 'Instead received: a DOM node.', - ); - expect(() => { - ReactTestUtils.scryRenderedComponentsWithType(true, 'span'); - }).toThrow( - 'The first argument must be a React class instance. ' + - 'Instead received: true.', - ); - expect(() => { - ReactTestUtils.findRenderedComponentWithType(true, 'span'); - }).toThrow( - 'The first argument must be a React class instance. ' + - 'Instead received: true.', - ); - }); - - describe('Simulate', () => { - // @gate !disableDOMTestUtils - it('should change the value of an input field', async () => { - const obj = { - handler: function (e) { - e.persist(); - }, - }; - spyOnDevAndProd(obj, 'handler'); - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(); - }); - const node = container.firstChild; - - node.value = 'giraffe'; - ReactTestUtils.Simulate.change(node); - - expect(obj.handler).toHaveBeenCalledWith( - expect.objectContaining({target: node}), - ); - }); - - // @gate !disableDOMTestUtils - it('should change the value of an input field in a component', async () => { - class SomeComponent extends React.Component { - inputRef = React.createRef(); - render() { - return ( -
- -
- ); - } - } - - const obj = { - handler: function (e) { - e.persist(); - }, - }; - spyOnDevAndProd(obj, 'handler'); - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let instance; - await act(() => { - root.render( - (instance = current)} - />, - ); - }); - - const node = instance.inputRef.current; - node.value = 'zebra'; - ReactTestUtils.Simulate.change(node); - - expect(obj.handler).toHaveBeenCalledWith( - expect.objectContaining({target: node}), - ); - }); - - // @gate !disableDOMTestUtils - it('should not warn when used with extra properties', async () => { - const CLIENT_X = 100; - - class Component extends React.Component { - childRef = React.createRef(); - handleClick = e => { - expect(e.clientX).toBe(CLIENT_X); - }; - - render() { - return
; - } - } - - const element = document.createElement('div'); - const root = ReactDOMClient.createRoot(element); - let instance; - await act(() => { - root.render( (instance = current)} />); - }); - - ReactTestUtils.Simulate.click(instance.childRef.current, { - clientX: CLIENT_X, - }); - }); - - // @gate !disableDOMTestUtils - it('should set the type of the event', async () => { - let event; - const stub = jest.fn().mockImplementation(e => { - e.persist(); - event = e; - }); - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let node; - await act(() => { - root.render(
(node = current)} />); - }); - - ReactTestUtils.Simulate.keyDown(node); - - expect(event.type).toBe('keydown'); - expect(event.nativeEvent.type).toBe('keydown'); - }); - - // @gate !disableDOMTestUtils - it('should work with renderIntoDocument', async () => { - const onChange = jest.fn(); - - class MyComponent extends React.Component { - render() { - return ( -
- -
- ); - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let instance; - await act(() => { - root.render( (instance = current)} />); - }); - - const input = ReactTestUtils.findRenderedDOMComponentWithTag( - instance, - 'input', - ); - input.value = 'giraffe'; - ReactTestUtils.Simulate.change(input); - - expect(onChange).toHaveBeenCalledWith( - expect.objectContaining({target: input}), - ); - }); - - // @gate !disableDOMTestUtils - it('should have mouse enter simulated by test utils', async () => { - const idCallOrder = []; - const recordID = function (id) { - idCallOrder.push(id); - }; - let CHILD; - function Child(props) { - return ( -
(CHILD = current)} - onMouseEnter={() => { - recordID(CHILD); - }} - /> - ); - } - - class ChildWrapper extends React.PureComponent { - render() { - return ; - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( -
-
- -
-
, - ); - }); - await act(() => { - ReactTestUtils.Simulate.mouseEnter(CHILD); - }); - expect(idCallOrder).toEqual([CHILD]); - }); - }); - - // @gate !disableDOMTestUtils - // @gate !disableLegacyMode - it('should call setState callback with no arguments', async () => { - let mockArgs; - class Component extends React.Component { - componentDidMount() { - this.setState({}, (...args) => (mockArgs = args)); - } - render() { - return false; - } - } - - ReactTestUtils.renderIntoDocument(); - - expect(mockArgs.length).toEqual(0); - }); - - // @gate !disableDOMTestUtils - it('should find rendered component with type in document', async () => { - class MyComponent extends React.Component { - render() { - return true; - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - let instance; - await act(() => { - root.render( (instance = current)} />); - }); - - const renderedComponentType = ReactTestUtils.findRenderedComponentWithType( - instance, - MyComponent, - ); - - expect(renderedComponentType).toBe(instance); - }); - - // @gate __DEV__ - it('warns when using `act`', () => { - expect(() => { - ReactTestUtils.act(() => {}); - }).toErrorDev( - [ - '`ReactDOMTestUtils.act` is deprecated in favor of `React.act`. ' + - 'Import `act` from `react` instead of `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ], - {withoutStack: true}, - ); - }); -}); diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsFB.js b/packages/react-dom/src/test-utils/ReactTestUtilsFB.js deleted file mode 100644 index 39ac99c3bc..0000000000 --- a/packages/react-dom/src/test-utils/ReactTestUtilsFB.js +++ /dev/null @@ -1,884 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @noflow - */ - -import * as React from 'react'; -import * as ReactDOM from 'react-dom'; -import {findCurrentFiberUsingSlowPath} from 'react-reconciler/src/ReactFiberTreeReflection'; -import {get as getInstance} from 'shared/ReactInstanceMap'; -import { - ClassComponent, - FunctionComponent, - HostComponent, - HostHoistable, - HostSingleton, - HostText, -} from 'react-reconciler/src/ReactWorkTags'; -import {SyntheticEvent} from 'react-dom-bindings/src/events/SyntheticEvent'; -import {ELEMENT_NODE} from 'react-dom-bindings/src/client/HTMLNodeType'; -import {disableDOMTestUtils} from 'shared/ReactFeatureFlags'; -import assign from 'shared/assign'; -import isArray from 'shared/isArray'; - -// Keep in sync with ReactDOM.js: -const SecretInternals = - ReactDOM.__DOM_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE; -const EventInternals = SecretInternals.Events; -const getInstanceFromNode = EventInternals[0]; -const getNodeFromInstance = EventInternals[1]; -const getFiberCurrentPropsFromNode = EventInternals[2]; -const enqueueStateRestore = EventInternals[3]; -const restoreStateIfNeeded = EventInternals[4]; - -let didWarnAboutUsingAct = false; -function act(callback) { - if (didWarnAboutUsingAct === false) { - didWarnAboutUsingAct = true; - console.error( - '`ReactDOMTestUtils.act` is deprecated in favor of `React.act`. ' + - 'Import `act` from `react` instead of `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - return React.act(callback); -} - -function Event(suffix) {} - -let hasWarnedAboutDeprecatedMockComponent = false; - -/** - * @class ReactTestUtils - */ - -function findAllInRenderedFiberTreeInternal(fiber, test) { - if (!fiber) { - return []; - } - const currentParent = findCurrentFiberUsingSlowPath(fiber); - if (!currentParent) { - return []; - } - let node = currentParent; - const ret = []; - while (true) { - if ( - node.tag === HostComponent || - node.tag === HostText || - node.tag === ClassComponent || - node.tag === FunctionComponent || - node.tag === HostHoistable || - node.tag === HostSingleton - ) { - const publicInst = node.stateNode; - if (test(publicInst)) { - ret.push(publicInst); - } - } - if (node.child) { - node.child.return = node; - node = node.child; - continue; - } - if (node === currentParent) { - return ret; - } - while (!node.sibling) { - if (!node.return || node.return === currentParent) { - return ret; - } - node = node.return; - } - node.sibling.return = node.return; - node = node.sibling; - } -} - -function validateClassInstance(inst, methodName) { - if (!inst) { - // This is probably too relaxed but it's existing behavior. - return; - } - if (getInstance(inst)) { - // This is a public instance indeed. - return; - } - let received; - const stringified = String(inst); - if (isArray(inst)) { - received = 'an array'; - } else if (inst && inst.nodeType === ELEMENT_NODE && inst.tagName) { - received = 'a DOM node'; - } else if (stringified === '[object Object]') { - received = 'object with keys {' + Object.keys(inst).join(', ') + '}'; - } else { - received = stringified; - } - - throw new Error( - `The first argument must be a React class instance. ` + - `Instead received: ${received}.`, - ); -} - -/** - * Utilities for making it easy to test React components. - * - * See https://reactjs.org/docs/test-utils.html - * - * Todo: Support the entire DOM.scry query syntax. For now, these simple - * utilities will suffice for testing purposes. - * @lends ReactTestUtils - */ -function renderIntoDocument(element) { - if (disableDOMTestUtils) { - throw new Error( - '`renderIntoDocument` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - const div = document.createElement('div'); - // None of our tests actually require attaching the container to the - // DOM, and doing so creates a mess that we rely on test isolation to - // clean up, so we're going to stop honoring the name of this method - // (and probably rename it eventually) if no problems arise. - // document.documentElement.appendChild(div); - return ReactDOM.render(element, div); -} - -function isElement(element) { - if (disableDOMTestUtils) { - throw new Error( - '`isElement` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - return React.isValidElement(element); -} - -function isElementOfType(inst, convenienceConstructor) { - if (disableDOMTestUtils) { - throw new Error( - '`isElementOfType` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - return React.isValidElement(inst) && inst.type === convenienceConstructor; -} - -function isDOMComponent(inst) { - if (disableDOMTestUtils) { - throw new Error( - '`isDOMComponent` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - return !!(inst && inst.nodeType === ELEMENT_NODE && inst.tagName); -} - -function isDOMComponentElement(inst) { - if (disableDOMTestUtils) { - throw new Error( - '`isDOMComponentElement` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - return !!(inst && React.isValidElement(inst) && !!inst.tagName); -} - -function isCompositeComponent(inst) { - if (disableDOMTestUtils) { - throw new Error( - '`isCompositeComponent` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - if (isDOMComponent(inst)) { - // Accessing inst.setState warns; just return false as that'll be what - // this returns when we have DOM nodes as refs directly - return false; - } - return ( - inst != null && - typeof inst.render === 'function' && - typeof inst.setState === 'function' - ); -} - -function isCompositeComponentWithType(inst, type) { - if (disableDOMTestUtils) { - throw new Error( - '`isCompositeComponentWithType` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - if (!isCompositeComponent(inst)) { - return false; - } - const internalInstance = getInstance(inst); - const constructor = internalInstance.type; - return constructor === type; -} - -function findAllInRenderedTree(inst, test) { - if (disableDOMTestUtils) { - throw new Error( - '`findAllInRenderedTree` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - validateClassInstance(inst, 'findAllInRenderedTree'); - if (!inst) { - return []; - } - const internalInstance = getInstance(inst); - return findAllInRenderedFiberTreeInternal(internalInstance, test); -} - -/** - * Finds all instances of components in the rendered tree that are DOM - * components with the class name matching `className`. - * @return {array} an array of all the matches. - */ -function scryRenderedDOMComponentsWithClass(root, classNames) { - if (disableDOMTestUtils) { - throw new Error( - '`scryRenderedDOMComponentsWithClass` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - validateClassInstance(root, 'scryRenderedDOMComponentsWithClass'); - return findAllInRenderedTree(root, function (inst) { - if (isDOMComponent(inst)) { - let className = inst.className; - if (typeof className !== 'string') { - // SVG, probably. - className = inst.getAttribute('class') || ''; - } - const classList = className.split(/\s+/); - - if (!isArray(classNames)) { - if (classNames === undefined) { - throw new Error( - 'TestUtils.scryRenderedDOMComponentsWithClass expects a ' + - 'className as a second argument.', - ); - } - - classNames = classNames.split(/\s+/); - } - return classNames.every(function (name) { - return classList.indexOf(name) !== -1; - }); - } - return false; - }); -} - -/** - * Like scryRenderedDOMComponentsWithClass but expects there to be one result, - * and returns that one result, or throws exception if there is any other - * number of matches besides one. - * @return {!ReactDOMComponent} The one match. - */ -function findRenderedDOMComponentWithClass(root, className) { - if (disableDOMTestUtils) { - throw new Error( - '`findRenderedDOMComponentWithClass` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - validateClassInstance(root, 'findRenderedDOMComponentWithClass'); - const all = scryRenderedDOMComponentsWithClass(root, className); - if (all.length !== 1) { - throw new Error( - 'Did not find exactly one match (found: ' + - all.length + - ') ' + - 'for class:' + - className, - ); - } - return all[0]; -} - -/** - * Finds all instances of components in the rendered tree that are DOM - * components with the tag name matching `tagName`. - * @return {array} an array of all the matches. - */ -function scryRenderedDOMComponentsWithTag(root, tagName) { - if (disableDOMTestUtils) { - throw new Error( - '`scryRenderedDOMComponentsWithTag` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - validateClassInstance(root, 'scryRenderedDOMComponentsWithTag'); - return findAllInRenderedTree(root, function (inst) { - return ( - isDOMComponent(inst) && - inst.tagName.toUpperCase() === tagName.toUpperCase() - ); - }); -} - -/** - * Like scryRenderedDOMComponentsWithTag but expects there to be one result, - * and returns that one result, or throws exception if there is any other - * number of matches besides one. - * @return {!ReactDOMComponent} The one match. - */ -function findRenderedDOMComponentWithTag(root, tagName) { - if (disableDOMTestUtils) { - throw new Error( - '`findRenderedDOMComponentWithTag` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - validateClassInstance(root, 'findRenderedDOMComponentWithTag'); - const all = scryRenderedDOMComponentsWithTag(root, tagName); - if (all.length !== 1) { - throw new Error( - 'Did not find exactly one match (found: ' + - all.length + - ') ' + - 'for tag:' + - tagName, - ); - } - return all[0]; -} - -/** - * Finds all instances of components with type equal to `componentType`. - * @return {array} an array of all the matches. - */ -function scryRenderedComponentsWithType(root, componentType) { - if (disableDOMTestUtils) { - throw new Error( - '`scryRenderedComponentsWithType` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - validateClassInstance(root, 'scryRenderedComponentsWithType'); - return findAllInRenderedTree(root, function (inst) { - return isCompositeComponentWithType(inst, componentType); - }); -} - -/** - * Same as `scryRenderedComponentsWithType` but expects there to be one result - * and returns that one result, or throws exception if there is any other - * number of matches besides one. - * @return {!ReactComponent} The one match. - */ -function findRenderedComponentWithType(root, componentType) { - if (disableDOMTestUtils) { - throw new Error( - '`findRenderedComponentWithType` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - validateClassInstance(root, 'findRenderedComponentWithType'); - const all = scryRenderedComponentsWithType(root, componentType); - if (all.length !== 1) { - throw new Error( - 'Did not find exactly one match (found: ' + - all.length + - ') ' + - 'for componentType:' + - componentType, - ); - } - return all[0]; -} - -/** - * Pass a mocked component module to this method to augment it with - * useful methods that allow it to be used as a dummy React component. - * Instead of rendering as usual, the component will become a simple - *
containing any provided children. - * - * @param {object} module the mock function object exported from a - * module that defines the component to be mocked - * @param {?string} mockTagName optional dummy root tag name to return - * from render method (overrides - * module.mockTagName if provided) - * @return {object} the ReactTestUtils object (for chaining) - */ -function mockComponent(module, mockTagName) { - if (disableDOMTestUtils) { - throw new Error( - '`mockComponent` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - if (__DEV__) { - if (!hasWarnedAboutDeprecatedMockComponent) { - hasWarnedAboutDeprecatedMockComponent = true; - console.warn( - 'ReactTestUtils.mockComponent() is deprecated. ' + - 'Use shallow rendering or jest.mock() instead.\n\n' + - 'See https://react.dev/link/test-utils-mock-component for more information.', - ); - } - } - - mockTagName = mockTagName || module.mockTagName || 'div'; - - module.prototype.render.mockImplementation(function () { - return React.createElement(mockTagName, null, this.props.children); - }); - - return this; -} - -function nativeTouchData(x, y) { - if (disableDOMTestUtils) { - throw new Error( - '`nativeTouchData` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - return { - touches: [{pageX: x, pageY: y}], - }; -} - -// Start of inline: the below functions were inlined from -// EventPropagator.js, as they deviated from ReactDOM's newer -// implementations. - -let hasError: boolean = false; -let caughtError: mixed = null; - -/** - * Dispatch the event to the listener. - * @param {SyntheticEvent} event SyntheticEvent to handle - * @param {function} listener Application-level callback - * @param {*} inst Internal component instance - */ -function executeDispatch(event, listener, inst) { - event.currentTarget = getNodeFromInstance(inst); - try { - listener(event); - } catch (error) { - if (!hasError) { - hasError = true; - caughtError = error; - } - } - event.currentTarget = null; -} - -/** - * Standard/simple iteration through an event's collected dispatches. - */ -function executeDispatchesInOrder(event) { - const dispatchListeners = event._dispatchListeners; - const dispatchInstances = event._dispatchInstances; - if (isArray(dispatchListeners)) { - for (let i = 0; i < dispatchListeners.length; i++) { - if (event.isPropagationStopped()) { - break; - } - // Listeners and Instances are two parallel arrays that are always in sync. - executeDispatch(event, dispatchListeners[i], dispatchInstances[i]); - } - } else if (dispatchListeners) { - executeDispatch(event, dispatchListeners, dispatchInstances); - } - event._dispatchListeners = null; - event._dispatchInstances = null; -} - -/** - * Dispatches an event and releases it back into the pool, unless persistent. - * - * @param {?object} event Synthetic event to be dispatched. - * @private - */ -function executeDispatchesAndRelease(event /* ReactSyntheticEvent */) { - if (event) { - executeDispatchesInOrder(event); - - if (!event.isPersistent()) { - event.constructor.release(event); - } - } -} - -function isInteractive(tag) { - return ( - tag === 'button' || - tag === 'input' || - tag === 'select' || - tag === 'textarea' - ); -} - -function getParent(inst) { - do { - inst = inst.return; - // TODO: If this is a HostRoot we might want to bail out. - // That is depending on if we want nested subtrees (layers) to bubble - // events to their parent. We could also go through parentNode on the - // host node but that wouldn't work for React Native and doesn't let us - // do the portal feature. - } while (inst && inst.tag !== HostComponent && inst.tag !== HostSingleton); - if (inst) { - return inst; - } - return null; -} - -/** - * Simulates the traversal of a two-phase, capture/bubble event dispatch. - */ -export function traverseTwoPhase(inst, fn, arg) { - const path = []; - while (inst) { - path.push(inst); - inst = getParent(inst); - } - let i; - for (i = path.length; i-- > 0; ) { - fn(path[i], 'captured', arg); - } - for (i = 0; i < path.length; i++) { - fn(path[i], 'bubbled', arg); - } -} - -function shouldPreventMouseEvent(name, type, props) { - switch (name) { - case 'onClick': - case 'onClickCapture': - case 'onDoubleClick': - case 'onDoubleClickCapture': - case 'onMouseDown': - case 'onMouseDownCapture': - case 'onMouseMove': - case 'onMouseMoveCapture': - case 'onMouseUp': - case 'onMouseUpCapture': - case 'onMouseEnter': - return !!(props.disabled && isInteractive(type)); - default: - return false; - } -} - -/** - * @param {object} inst The instance, which is the source of events. - * @param {string} registrationName Name of listener (e.g. `onClick`). - * @return {?function} The stored callback. - */ -function getListener(inst /* Fiber */, registrationName: string) { - // TODO: shouldPreventMouseEvent is DOM-specific and definitely should not - // live here; needs to be moved to a better place soon - const stateNode = inst.stateNode; - if (!stateNode) { - // Work in progress (ex: onload events in incremental mode). - return null; - } - const props = getFiberCurrentPropsFromNode(stateNode); - if (!props) { - // Work in progress. - return null; - } - const listener = props[registrationName]; - if (shouldPreventMouseEvent(registrationName, inst.type, props)) { - return null; - } - - if (listener && typeof listener !== 'function') { - throw new Error( - `Expected \`${registrationName}\` listener to be a function, instead got a value of \`${typeof listener}\` type.`, - ); - } - - return listener; -} - -function listenerAtPhase(inst, event, propagationPhase: PropagationPhases) { - let registrationName = event._reactName; - if (propagationPhase === 'captured') { - registrationName += 'Capture'; - } - return getListener(inst, registrationName); -} - -function accumulateDispatches(inst, ignoredDirection, event) { - if (inst && event && event._reactName) { - const registrationName = event._reactName; - const listener = getListener(inst, registrationName); - if (listener) { - if (event._dispatchListeners == null) { - event._dispatchListeners = []; - } - if (event._dispatchInstances == null) { - event._dispatchInstances = []; - } - event._dispatchListeners.push(listener); - event._dispatchInstances.push(inst); - } - } -} - -function accumulateDirectionalDispatches(inst, phase, event) { - if (__DEV__) { - if (!inst) { - console.error('Dispatching inst must not be null'); - } - } - const listener = listenerAtPhase(inst, event, phase); - if (listener) { - if (event._dispatchListeners == null) { - event._dispatchListeners = []; - } - if (event._dispatchInstances == null) { - event._dispatchInstances = []; - } - event._dispatchListeners.push(listener); - event._dispatchInstances.push(inst); - } -} - -function accumulateDirectDispatchesSingle(event) { - if (event && event._reactName) { - accumulateDispatches(event._targetInst, null, event); - } -} - -function accumulateTwoPhaseDispatchesSingle(event) { - if (event && event._reactName) { - traverseTwoPhase(event._targetInst, accumulateDirectionalDispatches, event); - } -} - -// End of inline - -const Simulate = {}; - -const directDispatchEventTypes = new Set([ - 'mouseEnter', - 'mouseLeave', - 'pointerEnter', - 'pointerLeave', -]); - -/** - * Exports: - * - * - `Simulate.click(Element)` - * - `Simulate.mouseMove(Element)` - * - `Simulate.change(Element)` - * - ... (All keys from event plugin `eventTypes` objects) - */ -function makeSimulator(eventType) { - return function (domNode, eventData) { - if (disableDOMTestUtils) { - throw new Error( - '`Simulate` was removed from `react-dom/test-utils`. ' + - 'See https://react.dev/warnings/react-dom-test-utils for more info.', - ); - } - - if (React.isValidElement(domNode)) { - throw new Error( - 'TestUtils.Simulate expected a DOM node as the first argument but received ' + - 'a React element. Pass the DOM node you wish to simulate the event on instead. ' + - 'Note that TestUtils.Simulate will not work if you are using shallow rendering.', - ); - } - - if (isCompositeComponent(domNode)) { - throw new Error( - 'TestUtils.Simulate expected a DOM node as the first argument but received ' + - 'a component instance. Pass the DOM node you wish to simulate the event on instead.', - ); - } - - const reactName = 'on' + eventType[0].toUpperCase() + eventType.slice(1); - const fakeNativeEvent = new Event(); - fakeNativeEvent.target = domNode; - fakeNativeEvent.type = eventType.toLowerCase(); - - const targetInst = getInstanceFromNode(domNode); - const event = new SyntheticEvent( - reactName, - fakeNativeEvent.type, - targetInst, - fakeNativeEvent, - domNode, - ); - - // Since we aren't using pooling, always persist the event. This will make - // sure it's marked and won't warn when setting additional properties. - event.persist(); - assign(event, eventData); - - if (directDispatchEventTypes.has(eventType)) { - accumulateDirectDispatchesSingle(event); - } else { - accumulateTwoPhaseDispatchesSingle(event); - } - - ReactDOM.unstable_batchedUpdates(function () { - // Normally extractEvent enqueues a state restore, but we'll just always - // do that since we're by-passing it here. - enqueueStateRestore(domNode); - executeDispatchesAndRelease(event); - if (hasError) { - const error = caughtError; - hasError = false; - caughtError = null; - throw error; - } - }); - restoreStateIfNeeded(); - }; -} - -// A one-time snapshot with no plans to update. We'll probably want to deprecate Simulate API. -const simulatedEventTypes = [ - 'blur', - 'cancel', - 'click', - 'close', - 'contextMenu', - 'copy', - 'cut', - 'auxClick', - 'doubleClick', - 'dragEnd', - 'dragStart', - 'drop', - 'focus', - 'input', - 'invalid', - 'keyDown', - 'keyPress', - 'keyUp', - 'mouseDown', - 'mouseUp', - 'paste', - 'pause', - 'play', - 'pointerCancel', - 'pointerDown', - 'pointerUp', - 'rateChange', - 'reset', - 'resize', - 'seeked', - 'submit', - 'touchCancel', - 'touchEnd', - 'touchStart', - 'volumeChange', - 'drag', - 'dragEnter', - 'dragExit', - 'dragLeave', - 'dragOver', - 'mouseMove', - 'mouseOut', - 'mouseOver', - 'pointerMove', - 'pointerOut', - 'pointerOver', - 'scroll', - 'toggle', - 'touchMove', - 'wheel', - 'abort', - 'animationEnd', - 'animationIteration', - 'animationStart', - 'canPlay', - 'canPlayThrough', - 'durationChange', - 'emptied', - 'encrypted', - 'ended', - 'error', - 'gotPointerCapture', - 'load', - 'loadedData', - 'loadedMetadata', - 'loadStart', - 'lostPointerCapture', - 'playing', - 'progress', - 'seeking', - 'stalled', - 'suspend', - 'timeUpdate', - 'transitionRun', - 'transitionStart', - 'transitionCancel', - 'transitionEnd', - 'waiting', - 'mouseEnter', - 'mouseLeave', - 'pointerEnter', - 'pointerLeave', - 'change', - 'select', - 'beforeInput', - 'beforeToggle', - 'compositionEnd', - 'compositionStart', - 'compositionUpdate', -]; -function buildSimulators() { - simulatedEventTypes.forEach(eventType => { - Simulate[eventType] = makeSimulator(eventType); - }); -} -buildSimulators(); - -export { - renderIntoDocument, - isElement, - isElementOfType, - isDOMComponent, - isDOMComponentElement, - isCompositeComponent, - isCompositeComponentWithType, - findAllInRenderedTree, - scryRenderedDOMComponentsWithClass, - findRenderedDOMComponentWithClass, - scryRenderedDOMComponentsWithTag, - findRenderedDOMComponentWithTag, - scryRenderedComponentsWithType, - findRenderedComponentWithType, - mockComponent, - nativeTouchData, - Simulate, - act, -}; diff --git a/packages/react-dom/test-utils.fb.js b/packages/react-dom/test-utils.fb.js deleted file mode 100644 index dc43cbed3f..0000000000 --- a/packages/react-dom/test-utils.fb.js +++ /dev/null @@ -1,10 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -export * from './src/test-utils/ReactTestUtilsFB'; diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 35a2e822e4..adec53c109 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -192,8 +192,6 @@ export const enableReactTestRendererWarning = true; // before removing them in stable in the next Major export const disableLegacyMode = true; -export const disableDOMTestUtils = true; - // Make equivalent to instead of export const enableRenderableContext = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 7ea580bd70..f5387abb03 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -98,8 +98,6 @@ export const disableStringRefs = true; export const enableReactTestRendererWarning = false; export const disableLegacyMode = false; -export const disableDOMTestUtils = false; - export const enableOwnerStacks = false; // Flow magic to verify the exports of this file match the original version. diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 7e16ee25b4..f6820d3bf5 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -22,7 +22,6 @@ export const enableRefAsProp = __TODO_NEXT_RN_MAJOR__; export const disableStringRefs = __TODO_NEXT_RN_MAJOR__; export const enableFastJSX = __TODO_NEXT_RN_MAJOR__; export const disableLegacyMode = __TODO_NEXT_RN_MAJOR__; -export const disableDOMTestUtils = __TODO_NEXT_RN_MAJOR__; export const useModernStrictMode = __TODO_NEXT_RN_MAJOR__; export const enableReactTestRendererWarning = __TODO_NEXT_RN_MAJOR__; export const enableAsyncActions = __TODO_NEXT_RN_MAJOR__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 4504d08fec..24d94adaf8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -92,7 +92,6 @@ export const disableStringRefs = true; export const enableFastJSX = true; export const disableLegacyMode = true; export const disableLegacyContext = true; -export const disableDOMTestUtils = true; export const enableRenderableContext = true; export const enableReactTestRendererWarning = true; export const disableDefaultPropsExceptForClasses = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 41aea40e76..731aa42147 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -85,7 +85,6 @@ export const enableFastJSX = true; export const enableReactTestRendererWarning = false; export const disableLegacyMode = false; -export const disableDOMTestUtils = false; export const disableDefaultPropsExceptForClasses = false; export const enableAddPropertiesFastPath = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 0aac888de9..9f5aa656c8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -85,7 +85,6 @@ export const enableFastJSX = false; export const enableReactTestRendererWarning = false; export const disableLegacyMode = false; -export const disableDOMTestUtils = false; export const disableDefaultPropsExceptForClasses = false; export const enableAddPropertiesFastPath = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index f135724f99..de8fdc2c0a 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -120,8 +120,6 @@ export const disableStringRefs = false; export const disableLegacyMode = __EXPERIMENTAL__; -export const disableDOMTestUtils = false; - export const enableOwnerStacks = false; // Flow magic to verify the exports of this file match the original version. diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index 09ffb11723..66e59124ca 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -231,7 +231,7 @@ const bundles = [ /******* Test Utils *******/ { moduleType: RENDERER_UTILS, - bundleTypes: [FB_WWW_DEV, NODE_DEV, NODE_PROD], + bundleTypes: [NODE_DEV, NODE_PROD], entry: 'react-dom/test-utils', global: 'ReactTestUtils', minifyWithProdErrorCodes: false, From 163122766b6008e992898b00f1fe3b104ed78737 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 28 May 2024 15:08:59 -0400 Subject: [PATCH 06/26] Fix: Use action implementation at time of dispatch (#29618) Fixes the behavior of actions that are queued by useActionState to use the action function that was current at the time it was dispatched, not at the time it eventually executes. The conceptual model is that the action is immediately dispatched, as if it were sent to a remote server/worker. It's the remote worker that maintains the queue, not the client. This is another property of actions makes them more like event handlers than like reducers. --- .../src/__tests__/ReactDOMForm-test.js | 47 ++++++++++++++++++- .../react-reconciler/src/ReactFiberHooks.js | 32 ++++++++----- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMForm-test.js b/packages/react-dom/src/__tests__/ReactDOMForm-test.js index f4dda30c45..9fa20e5d11 100644 --- a/packages/react-dom/src/__tests__/ReactDOMForm-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMForm-test.js @@ -1097,7 +1097,7 @@ describe('ReactDOMForm', () => { }); // @gate enableAsyncActions - test('queues multiple actions and runs them in order', async () => { + test('useActionState: queues multiple actions and runs them in order', async () => { let action; function App() { const [state, dispatch, isPending] = useActionState( @@ -1128,6 +1128,51 @@ describe('ReactDOMForm', () => { expect(container.textContent).toBe('D'); }); + // @gate enableAsyncActions + test( + 'useActionState: when calling a queued action, uses the implementation ' + + 'that was current at the time it was dispatched, not the most recent one', + async () => { + let action; + function App({throwIfActionIsDispatched}) { + const [state, dispatch, isPending] = useActionState(async (s, a) => { + if (throwIfActionIsDispatched) { + throw new Error('Oops!'); + } + return await getText(a); + }, 'Initial'); + action = dispatch; + return ; + } + + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + assertLog(['Initial']); + + // Dispatch two actions. The first one is async, so it forces the second + // one into an async queue. + await act(() => action('First action')); + assertLog(['Initial (pending)']); + // This action won't run until the first one finishes. + await act(() => action('Second action')); + + // While the first action is still pending, update a prop. This causes the + // inline action implementation to change, but it should not affect the + // behavior of the action that is already queued. + await act(() => root.render()); + assertLog(['Initial (pending)']); + + // Finish both of the actions. + await act(() => resolveText('First action')); + await act(() => resolveText('Second action')); + assertLog(['Second action']); + + // Confirm that if we dispatch yet another action, it uses the updated + // action implementation. + await expect(act(() => action('Third action'))).rejects.toThrow('Oops!'); + }, + ); + // @gate enableAsyncActions test('useActionState: works if action is sync', async () => { let increment; diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 3cb99b3519..e66e12c515 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1966,13 +1966,15 @@ type ActionStateQueue = { action: (Awaited, P) => S, // This is a circular linked list of pending action payloads. It incudes the // action that is currently running. - pending: ActionStateQueueNode

| null, + pending: ActionStateQueueNode | null, }; -type ActionStateQueueNode

= { +type ActionStateQueueNode = { payload: P, + // This is the action implementation at the time it was dispatched. + action: (Awaited, P) => S, // This is never null because it's part of a circular linked list. - next: ActionStateQueueNode

, + next: ActionStateQueueNode, }; function dispatchActionState( @@ -1989,8 +1991,9 @@ function dispatchActionState( if (last === null) { // There are no pending actions; this is the first one. We can run // it immediately. - const newLast: ActionStateQueueNode

= { + const newLast: ActionStateQueueNode = { payload, + action: actionQueue.action, next: (null: any), // circular }; newLast.next = actionQueue.pending = newLast; @@ -1999,13 +2002,14 @@ function dispatchActionState( actionQueue, (setPendingState: any), (setState: any), - payload, + newLast, ); } else { // There's already an action running. Add to the queue. const first = last.next; - const newLast: ActionStateQueueNode

= { + const newLast: ActionStateQueueNode = { payload, + action: actionQueue.action, next: first, }; actionQueue.pending = last.next = newLast; @@ -2016,11 +2020,8 @@ function runActionStateAction( actionQueue: ActionStateQueue, setPendingState: boolean => void, setState: Dispatch>, - payload: P, + node: ActionStateQueueNode, ) { - const action = actionQueue.action; - const prevState = actionQueue.state; - // This is a fork of startTransition const prevTransition = ReactSharedInternals.T; const currentTransition: BatchConfigTransition = {}; @@ -2033,6 +2034,15 @@ function runActionStateAction( // This will be reverted automatically when all actions are finished. setPendingState(true); + // `node.action` represents the action function at the time it was dispatched. + // If this action was queued, it might be stale, i.e. it's not necessarily the + // most current implementation of the action, stored on `actionQueue`. This is + // intentional. The conceptual model for queued actions is that they are + // queued in a remote worker; the dispatch happens immediately, only the + // execution is delayed. + const action = node.action; + const payload = node.payload; + const prevState = actionQueue.state; try { const returnValue = action(prevState, payload); const onStartTransitionFinish = ReactSharedInternals.S; @@ -2136,7 +2146,7 @@ function finishRunningActionStateAction( actionQueue, (setPendingState: any), (setState: any), - next.payload, + next, ); } } From 97722ca3d4675d544eb3753438aac70f01bbbf1f Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Tue, 28 May 2024 21:47:47 +0200 Subject: [PATCH 07/26] Export `version` from `react-dom` entry with `react-server` condition (#29596) --- packages/react-dom/src/ReactDOMReactServer.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react-dom/src/ReactDOMReactServer.js b/packages/react-dom/src/ReactDOMReactServer.js index 01aa77d759..50b3a61abd 100644 --- a/packages/react-dom/src/ReactDOMReactServer.js +++ b/packages/react-dom/src/ReactDOMReactServer.js @@ -8,6 +8,10 @@ */ // This is the subset of APIs that can be accessed from Server Component modules + +import ReactVersion from 'shared/ReactVersion'; +export {ReactVersion as version}; + export {default as __DOM_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE} from './ReactDOMSharedInternals'; export { prefetchDNS, From 46339720d75337ae1d1e113fd56ac99e7fd1a0b3 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 28 May 2024 12:12:15 -0700 Subject: [PATCH 08/26] compiler: error on reassigning to const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We currently don't report an error if the code attempts to reassign a const. Our thinking has been that we're not trying to catch all possible mistakes you could make in JavaScript — that's what ESLint, TypeScript, and Flow are for — and that we want to focus on React errors. However, accidentally reassigning a const is easy to catch and doesn't get in the way of other analysis so let's implement it. Note that React Compiler's ESLint plugin won't report these errors by default, but they will show up in playground. Fixes #29598 ghstack-source-id: a0af8b9a486d74a8991413322efddc3e3028c755 Pull Request resolved: https://github.com/facebook/react/pull/29619 --- .../src/HIR/BuildHIR.ts | 14 +++++++++++ .../src/HIR/HIR.ts | 3 ++- .../src/HIR/HIRBuilder.ts | 6 ++++- .../error.invalid-reassign-const.expect.md | 24 +++++++++++++++++++ .../compiler/error.invalid-reassign-const.js | 4 ++++ 5 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-const.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-const.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index 772fa61fbc..463881d2c4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -3336,6 +3336,20 @@ function lowerIdentifierForAssignment( }); return null; } + } else if ( + binding.bindingKind === "const" && + kind === InstructionKind.Reassign + ) { + builder.errors.push({ + reason: `Cannot reassign a \`const\` variable`, + severity: ErrorSeverity.InvalidJS, + loc: path.node.loc ?? null, + description: + binding.identifier.name != null + ? `\`${binding.identifier.name.value}\` is declared as const` + : null, + }); + return null; } const place: Place = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index af0e5afd28..a9cb55e39e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import { BindingKind } from "@babel/traverse"; import * as t from "@babel/types"; import { CompilerError, CompilerErrorDetailOptions } from "../CompilerError"; import { assertExhaustive } from "../Utils/utils"; @@ -1105,7 +1106,7 @@ export type MutableRange = { export type VariableBinding = // let, const, etc declared within the current component/hook - | { kind: "Identifier"; identifier: Identifier } + | { kind: "Identifier"; identifier: Identifier; bindingKind: BindingKind } // bindings declard outside the current component/hook | NonLocalBinding; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts index e8ba4a83b4..970e4ba51d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts @@ -280,7 +280,11 @@ export default class HIRBuilder { if (resolvedBinding.name && resolvedBinding.name.value !== originalName) { babelBinding.scope.rename(originalName, resolvedBinding.name.value); } - return { kind: "Identifier", identifier: resolvedBinding }; + return { + kind: "Identifier", + identifier: resolvedBinding, + bindingKind: babelBinding.kind, + }; } isContextIdentifier(path: NodePath): boolean { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-const.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-const.expect.md new file mode 100644 index 0000000000..adf45dad4f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-const.expect.md @@ -0,0 +1,24 @@ + +## Input + +```javascript +function Component() { + const x = 0; + x = 1; +} + +``` + + +## Error + +``` + 1 | function Component() { + 2 | const x = 0; +> 3 | x = 1; + | ^ InvalidJS: Cannot reassign a `const` variable. `x` is declared as const (3:3) + 4 | } + 5 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-const.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-const.js new file mode 100644 index 0000000000..d7443efa97 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-const.js @@ -0,0 +1,4 @@ +function Component() { + const x = 0; + x = 1; +} From bd4bb32fe708bace6fa927834220f571ff583f39 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 28 May 2024 15:09:34 -0700 Subject: [PATCH 09/26] compiler: fix for calls on builtin jsx/function types When I added new builtin types for jsx and functions, i forget to add a shape definition. This meant that attempting to accesss a property or method on these types would cause an internal error with an unresolved shape. That wasn't obvious because we rarely call methods on these types. I confirmed that the new fixtures here fail without the fix. ghstack-source-id: aa8f8d75a302bb5bac126d3e963594545e71ec74 Pull Request resolved: https://github.com/facebook/react/pull/29624 --- .../src/HIR/ObjectShape.ts | 3 + .../compiler/fbt/fbt-to-string.expect.md | 61 ++++++++++++ .../fixtures/compiler/fbt/fbt-to-string.js | 15 +++ ...pression-prototype-call-mutating.expect.md | 92 +++++++++++++++++++ ...tion-expression-prototype-call-mutating.js | 24 +++++ ...nction-expression-prototype-call.expect.md | 55 +++++++++++ .../function-expression-prototype-call.js | 11 +++ 7 files changed, 261 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-to-string.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-to-string.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call-mutating.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call-mutating.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index 995b3caa28..fd04bf43c2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -431,6 +431,9 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ ["*", { kind: "Object", shapeId: BuiltInMixedReadonlyId }], ]); +addObject(BUILTIN_SHAPES, BuiltInJsxId, []); +addObject(BUILTIN_SHAPES, BuiltInFunctionId, []); + export const DefaultMutatingHook = addHook( BUILTIN_SHAPES, { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-to-string.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-to-string.expect.md new file mode 100644 index 0000000000..192c16048b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-to-string.expect.md @@ -0,0 +1,61 @@ + +## Input + +```javascript +import fbt from "fbt"; + +function Component(props) { + const element = ( + + Hello {props.name} + + ); + return element.toString(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import fbt from "fbt"; + +function Component(props) { + const $ = _c(4); + let t0; + if ($[0] !== props.name) { + t0 = fbt._("Hello {user name}", [fbt._param("user name", props.name)], { + hk: "2zEDKF", + }); + $[0] = props.name; + $[1] = t0; + } else { + t0 = $[1]; + } + const element = t0; + let t1; + if ($[2] !== element) { + t1 = element.toString(); + $[2] = element; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +### Eval output +(kind: ok) "Hello Jason" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-to-string.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-to-string.js new file mode 100644 index 0000000000..3475a3262d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-to-string.js @@ -0,0 +1,15 @@ +import fbt from "fbt"; + +function Component(props) { + const element = ( + + Hello {props.name} + + ); + return element.toString(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call-mutating.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call-mutating.expect.md new file mode 100644 index 0000000000..86060212bc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call-mutating.expect.md @@ -0,0 +1,92 @@ + +## Input + +```javascript +import { useMemo } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +function Component(props) { + const a = useMemo(() => { + const a = []; + const f = function () { + a.push(props.name); + }; + f.call(); + return a; + }, [props.name]); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], + sequentialRenders: [ + { name: "Lauren" }, + { name: "Lauren" }, + { name: "Jason" }, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useMemo } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +function Component(props) { + const $ = _c(7); + let t0; + let a; + if ($[0] !== props.name) { + a = []; + const f = function () { + a.push(props.name); + }; + + f.call(); + $[0] = props.name; + $[1] = a; + } else { + a = $[1]; + } + t0 = a; + const a_0 = t0; + let t1; + if ($[2] !== props.name) { + t1 = [props.name]; + $[2] = props.name; + $[3] = t1; + } else { + t1 = $[3]; + } + let t2; + if ($[4] !== t1 || $[5] !== a_0) { + t2 = ; + $[4] = t1; + $[5] = a_0; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], + sequentialRenders: [ + { name: "Lauren" }, + { name: "Lauren" }, + { name: "Jason" }, + ], +}; + +``` + +### Eval output +(kind: ok)

{"inputs":["Lauren"],"output":["Lauren"]}
+
{"inputs":["Lauren"],"output":["Lauren"]}
+
{"inputs":["Jason"],"output":["Jason"]}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call-mutating.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call-mutating.js new file mode 100644 index 0000000000..2988e3e36c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call-mutating.js @@ -0,0 +1,24 @@ +import { useMemo } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +function Component(props) { + const a = useMemo(() => { + const a = []; + const f = function () { + a.push(props.name); + }; + f.call(); + return a; + }, [props.name]); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], + sequentialRenders: [ + { name: "Lauren" }, + { name: "Lauren" }, + { name: "Jason" }, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.expect.md new file mode 100644 index 0000000000..c9d46853f9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.expect.md @@ -0,0 +1,55 @@ + +## Input + +```javascript +function Component(props) { + const f = function () { + return
{props.name}
; + }; + return f.call(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(props) { + const $ = _c(4); + let t0; + if ($[0] !== props.name) { + t0 = function () { + return
{props.name}
; + }; + $[0] = props.name; + $[1] = t0; + } else { + t0 = $[1]; + } + const f = t0; + let t1; + if ($[2] !== f) { + t1 = f.call(); + $[2] = f; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +### Eval output +(kind: ok)
Jason
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.js new file mode 100644 index 0000000000..6a5e1ad922 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.js @@ -0,0 +1,11 @@ +function Component(props) { + const f = function () { + return
{props.name}
; + }; + return f.call(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; From 18164761b1ed4a0f70987ef56893285820549460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 28 May 2024 19:07:30 -0400 Subject: [PATCH 10/26] [Flight] Check if a return value is a client reference before introspecting (#29611) This didn't actually fail before but I'm just adding an extra check. Currently Client References are always "function" proxies so they never fall into this branch. However, we do in theory support objects as client references too depending on environment. We have checks elsewhere. So this just makes that consistent. --- .../src/__tests__/ReactFlightDOM-test.js | 26 +++++++++++++++++++ .../react-server/src/ReactFlightServer.js | 6 ++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index b792844a0b..5315b990d8 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -1828,4 +1828,30 @@ describe('ReactFlightDOM', () => { ); } }); + + it('should be able to render a client reference as return value', async () => { + const ClientModule = clientExports({ + text: 'Hello World', + }); + + function ServerComponent() { + return ClientModule.text; + } + + const {writable, readable} = getTestStream(); + const {pipe} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + pipe(writable); + const response = ReactServerDOMClient.createFromReadableStream(readable); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(response); + }); + expect(container.innerHTML).toBe('Hello World'); + }); }); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 59d005df8d..d8903597a8 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1021,7 +1021,11 @@ function renderFunctionComponent( const secondArg = undefined; result = Component(props, secondArg); } - if (typeof result === 'object' && result !== null) { + if ( + typeof result === 'object' && + result !== null && + !isClientReference(result) + ) { if (typeof result.then === 'function') { // When the return value is in children position we can resolve it immediately, // to its value without a wrapper if it's synchronously available. From a9a01068084550c0c71d8da222eb67eb7024c5b3 Mon Sep 17 00:00:00 2001 From: Niklas Mollenhauer Date: Wed, 29 May 2024 01:15:46 +0200 Subject: [PATCH 11/26] feat(compiler): Implement constant string concat propagation (#29621) ## Summary Resolves #29617 ## How did you test this change? I verified the implementation using the test. --- .../src/Optimization/ConstantPropagation.ts | 2 ++ ...nstant-propagation-string-concat.expect.md | 35 +++++++++++++++++++ .../constant-propagation-string-concat.js | 11 ++++++ 3 files changed, 48 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-string-concat.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-string-concat.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts index b9b16a93ff..04a401143a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts @@ -327,6 +327,8 @@ function evaluateInstruction( case "+": { if (typeof lhs === "number" && typeof rhs === "number") { result = { kind: "Primitive", value: lhs + rhs, loc: value.loc }; + } else if (typeof lhs === "string" && typeof rhs === "string") { + result = { kind: "Primitive", value: lhs + rhs, loc: value.loc }; } break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-string-concat.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-string-concat.expect.md new file mode 100644 index 0000000000..963d380ef3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-string-concat.expect.md @@ -0,0 +1,35 @@ + +## Input + +```javascript +function foo() { + const a = "a" + "b"; + const c = "c"; + return a + c; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; + +``` + +## Code + +```javascript +function foo() { + return "abc"; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; + +``` + +### Eval output +(kind: ok) "abc" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-string-concat.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-string-concat.js new file mode 100644 index 0000000000..052b223597 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-string-concat.js @@ -0,0 +1,11 @@ +function foo() { + const a = "a" + "b"; + const c = "c"; + return a + c; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; From 61aa159086b1a4f1dda987fc4b40e6396d05c5cd Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 29 May 2024 11:41:00 +0900 Subject: [PATCH 12/26] [compiler] Fix up prettier Our prettier setup is all messed up after the merge, so this PR should fix things ghstack-source-id: f825460ea6637138db6ba08fd6136fac3f7aa001 Pull Request resolved: https://github.com/facebook/react/pull/29213 --- .github/workflows/compiler-typescript.yml | 22 +++++- .../.eslintrc.js | 3 +- compiler/.prettierignore | 19 +++-- compiler/.prettierrc.js | 9 +++ compiler/.prettierrc.json | 4 - compiler/package.json | 3 + .../.prettierignore | 5 -- .../.prettierrc.json | 4 - .../babel-plugin-react-compiler/package.json | 4 - .../scripts/prettier.js | 79 ------------------- .../scripts/shared/list-changed-files.js | 39 --------- .../packages/make-read-only-util/package.json | 1 - compiler/packages/snap/package.json | 2 - compiler/packages/snap/src/compiler.ts | 10 +-- compiler/packages/snap/src/runner-worker.ts | 10 +-- compiler/yarn.lock | 7 +- 16 files changed, 59 insertions(+), 162 deletions(-) rename compiler/{packages/babel-plugin-react-compiler => }/.eslintrc.js (96%) create mode 100644 compiler/.prettierrc.js delete mode 100644 compiler/.prettierrc.json delete mode 100644 compiler/packages/babel-plugin-react-compiler/.prettierignore delete mode 100644 compiler/packages/babel-plugin-react-compiler/.prettierrc.json delete mode 100644 compiler/packages/babel-plugin-react-compiler/scripts/prettier.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/scripts/shared/list-changed-files.js diff --git a/.github/workflows/compiler-typescript.yml b/.github/workflows/compiler-typescript.yml index 1448f66961..4b4a668c72 100644 --- a/.github/workflows/compiler-typescript.yml +++ b/.github/workflows/compiler-typescript.yml @@ -24,6 +24,25 @@ jobs: run: echo "matrix=$(find packages -mindepth 1 -maxdepth 1 -type d | sed 's!packages/!!g' | tr '\n' ',' | sed s/.$// | jq -Rsc '. / "," - [""]')" >> $GITHUB_OUTPUT # Hardcoded to improve parallelism for babel-plugin-react-compiler + prettier: + name: Run prettier + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 18.x + cache: "yarn" + cache-dependency-path: compiler/yarn.lock + - name: Restore cached node_modules + uses: actions/cache@v4 + with: + path: "**/node_modules" + key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('compiler/**/yarn.lock') }} + - run: yarn install --frozen-lockfile + - run: yarn prettier:ci + + # Hardcoded to improve parallelism lint: name: Lint babel-plugin-react-compiler runs-on: ubuntu-latest @@ -40,10 +59,9 @@ jobs: path: "**/node_modules" key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('compiler/**/yarn.lock') }} - run: yarn install --frozen-lockfile - - run: yarn workspace babel-plugin-react-compiler prettier:ci - run: yarn workspace babel-plugin-react-compiler lint - # Hardcoded to improve parallelism for babel-plugin-react-compiler + # Hardcoded to improve parallelism jest: name: Jest babel-plugin-react-compiler runs-on: ubuntu-latest diff --git a/compiler/packages/babel-plugin-react-compiler/.eslintrc.js b/compiler/.eslintrc.js similarity index 96% rename from compiler/packages/babel-plugin-react-compiler/.eslintrc.js rename to compiler/.eslintrc.js index f669ee5d41..996046a130 100644 --- a/compiler/packages/babel-plugin-react-compiler/.eslintrc.js +++ b/compiler/.eslintrc.js @@ -82,11 +82,12 @@ module.exports = { ], "@typescript-eslint/array-type": ["error", { default: "generic" }], "@typescript-eslint/triple-slash-reference": "off", + "@typescript-eslint/no-var-requires": "off" }, parser: "@typescript-eslint/parser", plugins: ["@typescript-eslint"], root: true, - ignorePatterns: ["src/__tests__/**/*", "src/**/*.d.ts", "dist/**/*"], + ignorePatterns: ["**/__tests__/**/*", "**/*.d.ts", "**/dist/**/*"], env: { node: true, }, diff --git a/compiler/.prettierignore b/compiler/.prettierignore index 68457e6f8c..410e88836c 100644 --- a/compiler/.prettierignore +++ b/compiler/.prettierignore @@ -1,12 +1,21 @@ -.fixtures/ -bench/ **/dist **/__tests__/fixtures/**/*.expect.md **/__tests__/fixtures/**/*.flow.js **/.next -test262/ -*.md + +crates +apps/playground/public + +**/LICENSE +.* +*.md* *.json *.css *.webmanifest -packages/js-fuzzer \ No newline at end of file +*.map +*.sh +*.txt +*.ico +*.svg +*.lock +*.toml diff --git a/compiler/.prettierrc.js b/compiler/.prettierrc.js new file mode 100644 index 0000000000..37917d7082 --- /dev/null +++ b/compiler/.prettierrc.js @@ -0,0 +1,9 @@ +const config = { + requirePragma: false, + parser: "babel-ts", + semi: true, + singleQuote: false, + trailingComma: "es5" +} + +module.exports = config; diff --git a/compiler/.prettierrc.json b/compiler/.prettierrc.json deleted file mode 100644 index d2203e6dc6..0000000000 --- a/compiler/.prettierrc.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "requirePragma": false, - "parser": "babel-ts" -} diff --git a/compiler/package.json b/compiler/package.json index 76386be1d8..868d9b069b 100644 --- a/compiler/package.json +++ b/compiler/package.json @@ -24,6 +24,8 @@ "build": "yarn workspaces run build", "dev": "concurrently --kill-others -n compiler,runtime,playground \"yarn workspace babel-plugin-react-compiler run build --watch\" \"yarn workspace react-compiler-runtime run build --watch\" \"wait-on packages/babel-plugin-react-compiler/dist/index.js && yarn workspace playground run dev\"", "test": "yarn workspaces run test", + "prettier:write": "prettier --write . --log-level=warn", + "prettier:ci": "prettier --check . --log-level=warn", "snap": "yarn workspace babel-plugin-react-compiler run snap", "snap:build": "yarn workspace snap run build", "postinstall": "perl -p -i -e 's/react\\.element/react.transitional.element/' packages/snap/node_modules/fbt/lib/FbtReactUtil.js && perl -p -i -e 's/didWarnAboutUsingAct = false;/didWarnAboutUsingAct = true;/' packages/babel-plugin-react-compiler/node_modules/react-dom/cjs/react-dom-test-utils.development.js", @@ -40,6 +42,7 @@ "concurrently": "^7.4.0", "folder-hash": "^4.0.4", "ora": "5.4.1", + "prettier": "^3.2.5", "prompt-promise": "^1.0.3", "rollup": "^4.13.2", "rollup-plugin-banner2": "^1.2.3", diff --git a/compiler/packages/babel-plugin-react-compiler/.prettierignore b/compiler/packages/babel-plugin-react-compiler/.prettierignore deleted file mode 100644 index a1e3528290..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/.prettierignore +++ /dev/null @@ -1,5 +0,0 @@ -**/dist -**/__tests__/fixtures/**/*.expect.md -**/__tests__/fixtures/**/*.flow.js -*.md -*.json \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/.prettierrc.json b/compiler/packages/babel-plugin-react-compiler/.prettierrc.json deleted file mode 100644 index 9d725d0136..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/.prettierrc.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "requirePragma": false, - "parser": "babel-ts" -} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/package.json b/compiler/packages/babel-plugin-react-compiler/package.json index 746120e0ff..4d1d9f35a7 100644 --- a/compiler/packages/babel-plugin-react-compiler/package.json +++ b/compiler/packages/babel-plugin-react-compiler/package.json @@ -15,9 +15,6 @@ "snap:build": "yarn workspace snap run build", "snap:ci": "yarn snap:build && yarn snap", "ts:analyze-trace": "scripts/ts-analyze-trace.sh", - "prettier": "node ./scripts/prettier.js write-changed", - "prettier:all": "node ./scripts/prettier.js write", - "prettier:ci": "prettier --check .", "lint": "yarn eslint src" }, "dependencies": { @@ -53,7 +50,6 @@ "glob": "^7.1.6", "jest": "^29.0.3", "jest-environment-jsdom": "^29.0.3", - "prettier": "2.8.8", "react": "19.0.0-beta-b498834eab-20240506", "react-dom": "19.0.0-beta-b498834eab-20240506", "rimraf": "^3.0.2", diff --git a/compiler/packages/babel-plugin-react-compiler/scripts/prettier.js b/compiler/packages/babel-plugin-react-compiler/scripts/prettier.js deleted file mode 100644 index d5b8a1cda7..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/scripts/prettier.js +++ /dev/null @@ -1,79 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -"use strict"; - -/* - * Based on similar script in React - * https://github.com/facebook/react/blob/main/scripts/prettier/index.js - */ - -const chalk = require("chalk"); -const glob = require("glob"); -const prettier = require("prettier"); -const fs = require("fs"); -const listChangedFiles = require("./shared/list-changed-files"); -const prettierConfigPath = require.resolve("../.prettierrc"); - -const mode = process.argv[2] || "check"; -const shouldWrite = mode === "write" || mode === "write-changed"; -const onlyChanged = mode === "check-changed" || mode === "write-changed"; - -const changedFiles = onlyChanged ? listChangedFiles() : null; -let didWarn = false; -let didError = false; - -const files = glob - .sync("**/*.{js,ts,tsx,jsx}", { - ignore: ["**/node_modules/**", "**/__tests__/fixtures/**/*.flow.js"], - }) - .filter((f) => !onlyChanged || changedFiles.has(f)); -if (!files.length) { - return; -} - -files.forEach((file) => { - const options = prettier.resolveConfig.sync(file, { - config: prettierConfigPath, - }); - try { - const input = fs.readFileSync(file, "utf8"); - if (shouldWrite) { - const output = prettier.format(input, options); - if (output !== input) { - fs.writeFileSync(file, output, "utf8"); - } - } else { - if (!prettier.check(input, options)) { - if (!didWarn) { - console.log( - "\n" + - chalk.red( - ` This project uses prettier to format all JavaScript code.\n` - ) + - chalk.dim(` Please run `) + - chalk.reset("yarn prettier:all") + - chalk.dim( - ` and add changes to files listed below to your commit:` - ) + - `\n\n` - ); - didWarn = true; - } - console.log(file); - } - } - } catch (error) { - didError = true; - console.log("\n\n" + error.message); - console.log(file); - } -}); - -if (didWarn || didError) { - process.exitCode = 1; -} diff --git a/compiler/packages/babel-plugin-react-compiler/scripts/shared/list-changed-files.js b/compiler/packages/babel-plugin-react-compiler/scripts/shared/list-changed-files.js deleted file mode 100644 index f6a5e61bee..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/scripts/shared/list-changed-files.js +++ /dev/null @@ -1,39 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -"use strict"; - -const execFileSync = require("child_process").execFileSync; - -const exec = (command, args) => { - console.log("> " + [command].concat(args).join(" ")); - const options = { - cwd: process.cwd(), - env: process.env, - stdio: "pipe", - encoding: "utf-8", - }; - return execFileSync(command, args, options); -}; - -const execGitCmd = (args) => exec("git", args).trim().toString().split("\n"); - -const listChangedFiles = () => { - const mergeBase = execGitCmd(["merge-base", "HEAD", "main"]); - return new Set([ - ...execGitCmd([ - "diff", - "--name-only", - "--relative", - "--diff-filter=ACMRTUB", - mergeBase, - ]), - ...execGitCmd(["ls-files", "--others", "--exclude-standard"]), - ]); -}; - -module.exports = listChangedFiles; diff --git a/compiler/packages/make-read-only-util/package.json b/compiler/packages/make-read-only-util/package.json index e07616700c..fd79714bb4 100644 --- a/compiler/packages/make-read-only-util/package.json +++ b/compiler/packages/make-read-only-util/package.json @@ -17,7 +17,6 @@ "@types/jest": "^28.1.6", "@types/node": "^20.2.5", "jest": "^28.1.3", - "prettier": "2.8.8", "ts-jest": "^28.0.7", "ts-node": "^10.9.2" } diff --git a/compiler/packages/snap/package.json b/compiler/packages/snap/package.json index df3af415a8..60ccff029c 100644 --- a/compiler/packages/snap/package.json +++ b/compiler/packages/snap/package.json @@ -31,7 +31,6 @@ "glob": "^10.3.10", "hermes-parser": "^0.19.1", "jsdom": "^22.1.0", - "prettier": "2.8.8", "react": "19.0.0-beta-b498834eab-20240506", "react-dom": "19.0.0-beta-b498834eab-20240506", "readline": "^1.3.0", @@ -50,7 +49,6 @@ "@types/node": "^18.7.18", "@typescript-eslint/eslint-plugin": "^7.4.0", "@typescript-eslint/parser": "^7.4.0", - "prettier": "2.8.8", "rimraf": "^3.0.2" }, "resolutions": { diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 69b909645a..6c947273b1 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -271,8 +271,8 @@ function getEvaluatorPresets( ); return presets; } -function format(inputCode: string, language: "typescript" | "flow"): string { - return prettier.format(inputCode, { +async function format(inputCode: string, language: "typescript" | "flow"): Promise { + return await prettier.format(inputCode, { semi: true, parser: language === "typescript" ? "babel-ts" : "flow", }); @@ -288,13 +288,13 @@ export type TransformResult = { } | null; }; -export function transformFixtureInput( +export async function transformFixtureInput( input: string, fixturePath: string, parseConfigPragmaFn: typeof ParseConfigPragma, plugin: BabelCore.PluginObj, includeEvaluator: boolean -): { kind: "ok"; value: TransformResult } | { kind: "err"; msg: string } { +): Promise<{ kind: "ok"; value: TransformResult } | { kind: "err"; msg: string }> { // Extract the first line to quickly check for custom test directives const firstLine = input.substring(0, input.indexOf("\n")); @@ -398,7 +398,7 @@ export function transformFixtureInput( return { kind: "ok", value: { - forgetOutput: format(forgetOutput, language), + forgetOutput: await format(forgetOutput, language), evaluatorCode, }, }; diff --git a/compiler/packages/snap/src/runner-worker.ts b/compiler/packages/snap/src/runner-worker.ts index 47a0be00d6..54977f9b9a 100644 --- a/compiler/packages/snap/src/runner-worker.ts +++ b/compiler/packages/snap/src/runner-worker.ts @@ -33,16 +33,16 @@ export function clearRequireCache() { }); } -function compile( +async function compile( input: string, fixturePath: string, compilerVersion: number, shouldLog: boolean, includeEvaluator: boolean -): { +): Promise<{ error: string | null; compileResult: TransformResult | null; -} { +}> { const seenConsoleErrors: Array = []; console.error = (...messages: Array) => { seenConsoleErrors.push(...messages); @@ -68,7 +68,7 @@ function compile( // only try logging if we filtered out all but one fixture, // since console log order is non-deterministic toggleLogging(shouldLog); - const result = transformFixtureInput( + const result = await transformFixtureInput( input, fixturePath, parseConfigPragma, @@ -147,7 +147,7 @@ export async function transformFixture( unexpectedError: null, }; } - const { compileResult, error } = compile( + const { compileResult, error } = await compile( input, fixture.fixturePath, compilerVersion, diff --git a/compiler/yarn.lock b/compiler/yarn.lock index ff6485ad35..4e4c82e193 100644 --- a/compiler/yarn.lock +++ b/compiler/yarn.lock @@ -8423,16 +8423,11 @@ prelude-ls@~1.1.2: resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.1.2.tgz#21932a549f5e52ffd9a827f570e04be62a97da54" integrity sha512-ESF23V4SKG6lVSGZgYNpbsiaAkdab6ZgOxe52p7+Kid3W3u3bxR4Vfd/o21dmN7jSt0IwgZ4v5MUd26FEtXE9w== -prettier@*: +prettier@*, prettier@^3.2.5: version "3.2.5" resolved "https://registry.yarnpkg.com/prettier/-/prettier-3.2.5.tgz#e52bc3090586e824964a8813b09aba6233b28368" integrity sha512-3/GWa9aOC0YeD7LUfvOG2NiDyhOWRvt1k+rcKhOuYnMY24iiCphgneUfJDyFXd6rZCAnuLBv6UeAULtrhT/F4A== -prettier@2.8.8: - version "2.8.8" - resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.8.8.tgz#e8c5d7e98a4305ffe3de2e1fc4aca1a71c28b1da" - integrity sha512-tdN8qQGvNjw4CHbY+XXk0JgCXn9QiF21a55rBe5LJAU+kDyC4WQn4+awm2Xfk2lQMk5fKup9XgzTZtGkjBdP9Q== - prettier@3.0.3: version "3.0.3" resolved "https://registry.yarnpkg.com/prettier/-/prettier-3.0.3.tgz#432a51f7ba422d1469096c0fdc28e235db8f9643" From c998bb1ed4b3285398c9c7797135d3f060243c6a Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 29 May 2024 11:41:00 +0900 Subject: [PATCH 13/26] [compiler] Run prettier, fix snap After this is merged, I'll add it to .git-blame-ignore-revs. I can't do it now as the hash will change after ghstack lands this stack. ghstack-source-id: 054ca869b7839c589524c47d1962262f6b50f8ed Pull Request resolved: https://github.com/facebook/react/pull/29214 --- compiler/apps/playground/colors.js | 1 - .../components/Editor/EditorImpl.tsx | 18 +++++++-------- .../playground/components/Editor/Input.tsx | 2 +- .../playground/components/Editor/Output.tsx | 18 +++++++-------- .../playground/components/Editor/index.tsx | 2 +- .../components/Editor/monacoOptions.ts | 1 - .../apps/playground/components/Header.tsx | 2 +- .../components/Icons/IconGitHub.tsx | 2 +- compiler/apps/playground/components/Logo.tsx | 1 - .../playground/components/StoreContext.tsx | 1 - .../playground/components/TabbedWindow.tsx | 4 ++-- compiler/apps/playground/components/index.ts | 1 - compiler/apps/playground/hooks/index.ts | 1 - .../apps/playground/hooks/useMountEffect.ts | 1 - compiler/apps/playground/lib/createContext.ts | 1 - .../lib/reactCompilerMonacoDiagnostics.ts | 8 +++---- compiler/apps/playground/lib/stores/index.ts | 1 - .../apps/playground/lib/stores/messages.ts | 1 - compiler/apps/playground/next.config.js | 4 ++-- .../src/HIR/HIR.ts | 2 +- .../src/HIR/visitors.ts | 2 +- .../src/ReactiveScopes/visitors.ts | 2 +- ...cal-expression-instruction-scope.expect.md | 3 +-- .../compiler/capture-param-mutate.expect.md | 2 +- .../computed-call-evaluation-order.expect.md | 2 +- .../conditional-break-labeled.expect.md | 3 +-- .../conditional-early-return.expect.md | 3 +-- .../compiler/fbt/fbs-params.expect.md | 2 +- .../fbt-call-complex-param-value.expect.md | 2 +- .../fixtures/compiler/fbt/fbt-call.expect.md | 2 +- ...no-whitespace-btw-text-and-param.expect.md | 2 +- ...bt-param-with-leading-whitespace.expect.md | 4 ++-- ...t-param-with-trailing-whitespace.expect.md | 4 ++-- .../fbt-params-complex-param-value.expect.md | 2 +- .../fbt/fbt-preserve-jsxtext.expect.md | 4 ++-- .../fbt/fbt-preserve-whitespace.expect.md | 4 ++-- ...-single-space-btw-param-and-text.expect.md | 2 +- ...bt-whitespace-around-param-value.expect.md | 2 +- .../fbt/fbt-whitespace-within-text.expect.md | 2 +- ...btparam-with-jsx-element-content.expect.md | 8 +++---- ...fbtparam-with-jsx-fragment-value.expect.md | 2 +- .../compiler/fbt/lambda-with-fbt.expect.md | 2 +- .../flag-enable-emit-hook-guards.expect.md | 2 +- .../fixtures/compiler/for-of-mutate.expect.md | 2 +- .../fixtures/compiler/hook-noAlias.expect.md | 2 +- .../fixtures/compiler/independent.expect.md | 3 +-- ...ion-expression-React-memo-gating.expect.md | 2 +- .../compiler/interdependent.expect.md | 3 +-- ...ith-independently-memoizable-arg.expect.md | 2 +- ...ject-method-calls-mutable-lambda.expect.md | 2 +- .../readonly-object-method-calls.expect.md | 2 +- ...reactive-scope-with-early-return.expect.md | 2 +- ...erge-overlapping-reactive-scopes.expect.md | 2 +- .../make-read-only-util/src/makeReadOnly.ts | 6 ++++- .../src/checks/reactCompiler.ts | 8 +++---- compiler/packages/snap/src/compiler.ts | 9 ++++++-- compiler/scripts/release/publish-manual.js | 22 +++++++++---------- 57 files changed, 97 insertions(+), 105 deletions(-) diff --git a/compiler/apps/playground/colors.js b/compiler/apps/playground/colors.js index fb9f181af5..7259e1f32b 100644 --- a/compiler/apps/playground/colors.js +++ b/compiler/apps/playground/colors.js @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ - /** * Sync from . */ diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index 340a7cd0a7..e8a1177b4e 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -43,7 +43,7 @@ import { } from "./Output"; function parseFunctions( - source: string, + source: string ): Array< NodePath< t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression @@ -148,7 +148,7 @@ function isHookName(s: string): boolean { } function getReactFunctionType( - id: NodePath, + id: NodePath ): ReactFunctionType { if (id && id.node && id.isIdentifier()) { if (isHookName(id.node.name)) { @@ -189,7 +189,7 @@ function compile(source: string): CompilerOutput { severity: ErrorSeverity.Todo, loc: fn.node.loc ?? null, suggestions: null, - }), + }) ); continue; } @@ -205,7 +205,7 @@ function compile(source: string): CompilerOutput { "_c", null, null, - null, + null )) { const fnName = fn.node.id?.name ?? null; switch (result.kind) { @@ -274,7 +274,7 @@ function compile(source: string): CompilerOutput { reason: `Unexpected failure when transforming input! ${err}`, loc: null, suggestions: null, - }), + }) ); } } @@ -291,7 +291,7 @@ export default function Editor() { const { enqueueSnackbar } = useSnackbar(); const compilerOutput = useMemo( () => compile(deferredStore.source), - [deferredStore.source], + [deferredStore.source] ); useMountEffect(() => { @@ -305,7 +305,7 @@ export default function Editor() { ...createMessage( "Bad URL - fell back to the default Playground.", MessageLevel.Info, - MessageSource.Playground, + MessageSource.Playground ), }); mountStore = defaultStore; @@ -319,9 +319,7 @@ export default function Editor() { return ( <>
-
+
, + > ); lastPassOutput = text; } @@ -122,7 +122,7 @@ async function tabify(source: string, compilerOutput: CompilerOutput) { output={code} diff={null} showInfoPanel={false} - >, + > ); if (sourceMapUrl) { reorderedTabs.set( @@ -133,7 +133,7 @@ async function tabify(source: string, compilerOutput: CompilerOutput) { className="w-full h-monaco_small sm:h-monaco" title="Generated Code" /> - , + ); } } @@ -145,16 +145,16 @@ async function tabify(source: string, compilerOutput: CompilerOutput) { async function codegen( ast: t.Program, - source: string, + source: string ): Promise<{ code: any; sourceMapUrl: string | null }> { const generated = generate( ast, { sourceMaps: true, sourceFileName: "input.js" }, - source, + source ); const sourceMapUrl = getSourceMapUrl( generated.code, - JSON.stringify(generated.map), + JSON.stringify(generated.map) ); const codegenOutput = await prettier.format(generated.code, { semi: true, @@ -172,14 +172,14 @@ function getSourceMapUrl(code: string, map: string): string | null { code = utf16ToUTF8(code); map = utf16ToUTF8(map); return `https://evanw.github.io/source-map-visualization/#${btoa( - `${code.length}\0${code}${map.length}\0${map}`, + `${code.length}\0${code}${map.length}\0${map}` )}`; } function Output({ store, compilerOutput }: Props) { - const [tabsOpen, setTabsOpen] = useState>(() => new Set(['JS'])); + const [tabsOpen, setTabsOpen] = useState>(() => new Set(["JS"])); const [tabs, setTabs] = useState>( - () => new Map(), + () => new Map() ); useEffect(() => { tabify(store.source, compilerOutput).then((tabs) => { diff --git a/compiler/apps/playground/components/Editor/index.tsx b/compiler/apps/playground/components/Editor/index.tsx index 8a32f962bc..81b3b75e34 100644 --- a/compiler/apps/playground/components/Editor/index.tsx +++ b/compiler/apps/playground/components/Editor/index.tsx @@ -13,4 +13,4 @@ const Editor = dynamic(() => import("./EditorImpl"), { ssr: false, }); -export default Editor; \ No newline at end of file +export default Editor; diff --git a/compiler/apps/playground/components/Editor/monacoOptions.ts b/compiler/apps/playground/components/Editor/monacoOptions.ts index df6fccef45..d5ce32039c 100644 --- a/compiler/apps/playground/components/Editor/monacoOptions.ts +++ b/compiler/apps/playground/components/Editor/monacoOptions.ts @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ - import type { EditorProps } from "@monaco-editor/react"; export const monacoOptions: Partial = { diff --git a/compiler/apps/playground/components/Header.tsx b/compiler/apps/playground/components/Header.tsx index c8996a7494..2588e6d25c 100644 --- a/compiler/apps/playground/components/Header.tsx +++ b/compiler/apps/playground/components/Header.tsx @@ -50,7 +50,7 @@ export default function Header() {

React Compiler Playground

diff --git a/compiler/apps/playground/components/Icons/IconGitHub.tsx b/compiler/apps/playground/components/Icons/IconGitHub.tsx index af3b54fda0..b96d6a6aa7 100644 --- a/compiler/apps/playground/components/Icons/IconGitHub.tsx +++ b/compiler/apps/playground/components/Icons/IconGitHub.tsx @@ -21,5 +21,5 @@ export const IconGitHub = memo( ); - }, + } ); diff --git a/compiler/apps/playground/components/Logo.tsx b/compiler/apps/playground/components/Logo.tsx index 54ad5fffa6..07ab2bd265 100644 --- a/compiler/apps/playground/components/Logo.tsx +++ b/compiler/apps/playground/components/Logo.tsx @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ - // https://github.com/reactjs/reactjs.org/blob/main/beta/src/components/Logo.tsx export default function Logo(props: JSX.IntrinsicElements["svg"]) { diff --git a/compiler/apps/playground/components/StoreContext.tsx b/compiler/apps/playground/components/StoreContext.tsx index 09dafae813..2f533c50c5 100644 --- a/compiler/apps/playground/components/StoreContext.tsx +++ b/compiler/apps/playground/components/StoreContext.tsx @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ - import type { Dispatch, ReactNode } from "react"; import { useReducer } from "react"; import createContext from "../lib/createContext"; diff --git a/compiler/apps/playground/components/TabbedWindow.tsx b/compiler/apps/playground/components/TabbedWindow.tsx index 29d094298e..6d7ed3517b 100644 --- a/compiler/apps/playground/components/TabbedWindow.tsx +++ b/compiler/apps/playground/components/TabbedWindow.tsx @@ -78,7 +78,7 @@ function TabbedWindowItem({ title="Minimize tab" aria-label="Minimize tab" onClick={toggleTabs} - className={`p-4 duration-150 ease-in border-b cursor-pointer border-grey-200 ${hasChanged ? 'font-bold' : 'font-light'} text-secondary hover:text-link`} + className={`p-4 duration-150 ease-in border-b cursor-pointer border-grey-200 ${hasChanged ? "font-bold" : "font-light"} text-secondary hover:text-link`} > - {name} @@ -91,7 +91,7 @@ function TabbedWindowItem({ aria-label={`Expand compiler tab: ${name}`} style={{ transform: "rotate(90deg) translate(-50%)" }} onClick={toggleTabs} - className={`flex-grow-0 w-5 transition-colors duration-150 ease-in ${hasChanged ? 'font-bold' : 'font-light'} text-secondary hover:text-link`} + className={`flex-grow-0 w-5 transition-colors duration-150 ease-in ${hasChanged ? "font-bold" : "font-light"} text-secondary hover:text-link`} > {name} diff --git a/compiler/apps/playground/components/index.ts b/compiler/apps/playground/components/index.ts index 283a7c74cc..b4018dace3 100644 --- a/compiler/apps/playground/components/index.ts +++ b/compiler/apps/playground/components/index.ts @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ - export { default as Editor } from "./Editor"; export { default as Header } from "./Header"; export { StoreProvider } from "./StoreContext"; diff --git a/compiler/apps/playground/hooks/index.ts b/compiler/apps/playground/hooks/index.ts index 2cca4a9007..04ad9063af 100644 --- a/compiler/apps/playground/hooks/index.ts +++ b/compiler/apps/playground/hooks/index.ts @@ -5,5 +5,4 @@ * LICENSE file in the root directory of this source tree. */ - export { default as useMountEffect } from "./useMountEffect"; diff --git a/compiler/apps/playground/hooks/useMountEffect.ts b/compiler/apps/playground/hooks/useMountEffect.ts index b08003bcf7..a37e053513 100644 --- a/compiler/apps/playground/hooks/useMountEffect.ts +++ b/compiler/apps/playground/hooks/useMountEffect.ts @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ - import type { EffectCallback } from "react"; import { useEffect } from "react"; diff --git a/compiler/apps/playground/lib/createContext.ts b/compiler/apps/playground/lib/createContext.ts index c6f129b7d2..f9e00ee90a 100644 --- a/compiler/apps/playground/lib/createContext.ts +++ b/compiler/apps/playground/lib/createContext.ts @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ - import React from "react"; /** diff --git a/compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts b/compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts index 9a7084cd69..b5dbb03d5c 100644 --- a/compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts +++ b/compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts @@ -14,7 +14,7 @@ import { MarkerSeverity, type editor } from "monaco-editor"; function mapReactCompilerSeverityToMonaco( level: ErrorSeverity, - monaco: Monaco, + monaco: Monaco ): MarkerSeverity { switch (level) { case ErrorSeverity.Todo: @@ -26,7 +26,7 @@ function mapReactCompilerSeverityToMonaco( function mapReactCompilerDiagnosticToMonacoMarker( detail: CompilerErrorDetail, - monaco: Monaco, + monaco: Monaco ): editor.IMarkerData | null { if (detail.loc == null || typeof detail.loc === "symbol") { return null; @@ -70,7 +70,7 @@ export function renderReactCompilerMarkers({ marker.startLineNumber, marker.startColumn, marker.endLineNumber, - marker.endColumn, + marker.endColumn ), options: { isWholeLine: true, @@ -83,7 +83,7 @@ export function renderReactCompilerMarkers({ monaco.editor.setModelMarkers(model, "owner", []); decorations = model.deltaDecorations( model.getAllDecorations().map((d) => d.id), - [], + [] ); } } diff --git a/compiler/apps/playground/lib/stores/index.ts b/compiler/apps/playground/lib/stores/index.ts index 880ec1c2fd..aa00e375f1 100644 --- a/compiler/apps/playground/lib/stores/index.ts +++ b/compiler/apps/playground/lib/stores/index.ts @@ -5,6 +5,5 @@ * LICENSE file in the root directory of this source tree. */ - export * from "./messages"; export * from "./store"; diff --git a/compiler/apps/playground/lib/stores/messages.ts b/compiler/apps/playground/lib/stores/messages.ts index f3f1495783..7c791386c5 100644 --- a/compiler/apps/playground/lib/stores/messages.ts +++ b/compiler/apps/playground/lib/stores/messages.ts @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ - export enum MessageSource { Babel, Forget, diff --git a/compiler/apps/playground/next.config.js b/compiler/apps/playground/next.config.js index 4d00624b35..ddb2f958ca 100644 --- a/compiler/apps/playground/next.config.js +++ b/compiler/apps/playground/next.config.js @@ -23,7 +23,7 @@ const nextConfig = { new MonacoWebpackPlugin({ languages: ["typescript", "javascript"], filename: "static/[name].worker.js", - }), + }) ); } @@ -31,7 +31,7 @@ const nextConfig = { ...config.resolve.alias, "react-compiler-runtime": path.resolve( __dirname, - "../../packages/react-compiler-runtime", + "../../packages/react-compiler-runtime" ), }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index a9cb55e39e..da900c275c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -78,7 +78,7 @@ export type ReactiveInstructionStatement = { }; export type ReactiveTerminalStatement< - Tterminal extends ReactiveTerminal = ReactiveTerminal + Tterminal extends ReactiveTerminal = ReactiveTerminal, > = { kind: "terminal"; terminal: Tterminal; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts index e6e5878cc0..aaf3ffc4c5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts @@ -876,7 +876,7 @@ export function mapTerminalSuccessors( export function terminalHasFallthrough< T extends Terminal, - U extends T & { fallthrough: BlockId } + U extends T & { fallthrough: BlockId }, >(terminal: T): terminal is U { switch (terminal.kind) { case "maybe-throw": diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/visitors.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/visitors.ts index 2e2fd1b5d0..4d40f6537b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/visitors.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/visitors.ts @@ -257,7 +257,7 @@ export type Transformed = | { kind: "replace-many"; value: Array }; export class ReactiveFunctionTransform< - TState = void + TState = void, > extends ReactiveFunctionVisitor { override traverseBlock(block: ReactiveBlock, state: TState): void { let nextBlock: ReactiveBlock | null = null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allocating-logical-expression-instruction-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allocating-logical-expression-instruction-scope.expect.md index 84887a36c6..f7bf45db86 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allocating-logical-expression-instruction-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allocating-logical-expression-instruction-scope.expect.md @@ -24,8 +24,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; -/** +import { c as _c } from "react/compiler-runtime"; /** * This is a weird case as data has type `BuiltInMixedReadonly`. * The only scoped value we currently infer in this program is the * PropertyLoad `data?.toString`. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-param-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-param-mutate.expect.md index 3e3c262220..447ca19e21 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-param-mutate.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-param-mutate.expect.md @@ -76,7 +76,7 @@ function getNativeLogFunction(level) { INSPECTOR_LEVELS[logLevel], str, [].slice.call(arguments), - INSPECTOR_FRAMES_TO_SKIP + INSPECTOR_FRAMES_TO_SKIP, ); } if (groupStack.length) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/computed-call-evaluation-order.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/computed-call-evaluation-order.expect.md index 3ea5625688..76bdcf1efe 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/computed-call-evaluation-order.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/computed-call-evaluation-order.expect.md @@ -46,7 +46,7 @@ function Component() { x = { f: () => console.log("original") }; (console.log("A"), x)[(console.log("B"), "f")]( - (changeF(x), console.log("arg"), 1) + (changeF(x), console.log("arg"), 1), ); $[1] = x; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md index 5160f9fecb..2c6b373f0c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md @@ -29,8 +29,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; -/** +import { c as _c } from "react/compiler-runtime"; /** * props.b *does* influence `a` */ function Component(props) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-early-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-early-return.expect.md index 1cc33444dc..04012db573 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-early-return.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-early-return.expect.md @@ -66,8 +66,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; -/** +import { c as _c } from "react/compiler-runtime"; /** * props.b does *not* influence `a` */ function ComponentA(props) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbs-params.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbs-params.expect.md index 68c74a3c04..d2a4c665af 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbs-params.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbs-params.expect.md @@ -40,7 +40,7 @@ function Component(props) { title={fbs._( "Hello {user name}", [fbs._param("user name", props.name)], - { hk: "2zEDKF" } + { hk: "2zEDKF" }, )} > Hover me diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-call-complex-param-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-call-complex-param-value.expect.md index 0b76eb43e0..69f37660b7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-call-complex-param-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-call-complex-param-value.expect.md @@ -34,7 +34,7 @@ function Component(props) { t0 = fbt._( "Hello, {(key) name}!", [fbt._param("(key) name", identity(props.name))], - { hk: "2sOsn5" } + { hk: "2sOsn5" }, ); $[0] = props.name; $[1] = t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-call.expect.md index ca0d760534..dde6d80014 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-call.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-call.expect.md @@ -32,7 +32,7 @@ function Component(props) { t0 = fbt._( "{(key) count} items", [fbt._param("(key) count", props.count)], - { hk: "3yW91j" } + { hk: "3yW91j" }, ); $[0] = props.count; $[1] = t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-no-whitespace-btw-text-and-param.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-no-whitespace-btw-text-and-param.expect.md index a080b44870..b48c92bb76 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-no-whitespace-btw-text-and-param.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-no-whitespace-btw-text-and-param.expect.md @@ -35,7 +35,7 @@ function Component(t0) { t1 = fbt._( "Before text{paramName}After text", [fbt._param("paramName", value)], - { hk: "aKEGX" } + { hk: "aKEGX" }, ); $[0] = value; $[1] = t1; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-leading-whitespace.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-leading-whitespace.expect.md index 1b7fa4353b..a62bbbcdf9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-leading-whitespace.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-leading-whitespace.expect.md @@ -56,10 +56,10 @@ function Component(props) { fbt._param( "option", - props.option + props.option, ), ], - { hk: "3Bg20a" } + { hk: "3Bg20a" }, )} ! diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-trailing-whitespace.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-trailing-whitespace.expect.md index 6c2a7e8572..3f5fdf9a2b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-trailing-whitespace.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-trailing-whitespace.expect.md @@ -56,10 +56,10 @@ function Component(props) { fbt._param( "option", - props.option + props.option, ), ], - { hk: "3Bg20a" } + { hk: "3Bg20a" }, )} ! diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-params-complex-param-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-params-complex-param-value.expect.md index 2bfd419ada..44461081d7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-params-complex-param-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-params-complex-param-value.expect.md @@ -27,7 +27,7 @@ function Component(props) { t0 = fbt._( "Hello {user name}", [fbt._param("user name", capitalize(props.name))], - { hk: "2zEDKF" } + { hk: "2zEDKF" }, ); $[0] = props.name; $[1] = t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.expect.md index f4eeb80bb4..d2f10350bc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.expect.md @@ -36,10 +36,10 @@ function Foo(props) { fbt._param( "value", - props.value + props.value, ), ], - { hk: "Ri5kJ" } + { hk: "Ri5kJ" }, ); $[0] = props.value; $[1] = t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace.expect.md index 995f9c6ffc..5e67bbc163 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace.expect.md @@ -39,10 +39,10 @@ function Component(t0) { fbt._param( "paramName", - value + value, ), ], - { hk: "3z5SVE" } + { hk: "3z5SVE" }, ); $[0] = value; $[1] = t1; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-single-space-btw-param-and-text.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-single-space-btw-param-and-text.expect.md index 4d29c6650c..7fa312c49f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-single-space-btw-param-and-text.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-single-space-btw-param-and-text.expect.md @@ -35,7 +35,7 @@ function Component(t0) { t1 = fbt._( "Before text {paramName} after text", [fbt._param("paramName", value)], - { hk: "26pxNm" } + { hk: "26pxNm" }, ); $[0] = value; $[1] = t1; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-whitespace-around-param-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-whitespace-around-param-value.expect.md index bd62994eaa..3bfd3bab52 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-whitespace-around-param-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-whitespace-around-param-value.expect.md @@ -35,7 +35,7 @@ function Component(t0) { t1 = fbt._( "Before text {paramName} after text", [fbt._param("paramName", value)], - { hk: "26pxNm" } + { hk: "26pxNm" }, ); $[0] = value; $[1] = t1; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-whitespace-within-text.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-whitespace-within-text.expect.md index 16aecf4580..55326204ca 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-whitespace-within-text.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-whitespace-within-text.expect.md @@ -37,7 +37,7 @@ function Component(t0) { t1 = fbt._( "Before text {paramName} after text more text and more and more and more and more and more and more and more and more and blah blah blah blah", [fbt._param("paramName", value)], - { hk: "24ZPpO" } + { hk: "24ZPpO" }, ); $[0] = value; $[1] = t1; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md index fd0c5e72e3..387031fc18 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md @@ -41,12 +41,12 @@ function Component(t0) { fbt._param( "item author", - {name} + {name}, ), fbt._param( "icon", - icon + icon, ), fbt._implicitParam( "=m2", @@ -54,10 +54,10 @@ function Component(t0) { {fbt._("{item details}", [fbt._param("item details", data)], { hk: "4jLfVq", })} - + , ), ], - { hk: "2HLm2j" } + { hk: "2HLm2j" }, )} ); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-fragment-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-fragment-value.expect.md index 20c916cbff..210bad8cf1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-fragment-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-fragment-value.expect.md @@ -35,7 +35,7 @@ function Component(props) { value={fbt._( "{value}%", [fbt._param("value", <>{identity(props.text)})], - { hk: "10F5Cc" } + { hk: "10F5Cc" }, )} /> ); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/lambda-with-fbt.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/lambda-with-fbt.expect.md index bb85e2c0be..5ac0ad17a0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/lambda-with-fbt.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/lambda-with-fbt.expect.md @@ -57,7 +57,7 @@ function Component() { return fbt._( "Gift | {price}", [fbt._param("price", item?.current_gift_offer?.price?.formatted)], - { hk: "3GTnGE" } + { hk: "3GTnGE" }, ); } else { if (!iconOnly && !showPrice) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/flag-enable-emit-hook-guards.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/flag-enable-emit-hook-guards.expect.md index e49b3a4189..b3a98ab626 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/flag-enable-emit-hook-guards.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/flag-enable-emit-hook-guards.expect.md @@ -107,7 +107,7 @@ function Component(t0) { } finally { $dispatcherGuard(3); } - })() + })(), ); } finally { $dispatcherGuard(3); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-of-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-of-mutate.expect.md index 98f132d44f..e8175be98e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-of-mutate.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-of-mutate.expect.md @@ -35,7 +35,7 @@ function Component(_props) { const results = []; for (const item of collection) { results.push( -
{toJSON(mutateAndReturn(item))}
+
{toJSON(mutateAndReturn(item))}
, ); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-noAlias.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-noAlias.expect.md index 4010d00699..b93b26d141 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-noAlias.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-noAlias.expect.md @@ -46,7 +46,7 @@ function Component(props) { () => { console.log(props); }, - [props.a] + [props.a], ); let t1; if ($[2] !== x || $[3] !== item) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/independent.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/independent.expect.md index b981b0adaa..ac8a952e78 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/independent.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/independent.expect.md @@ -27,8 +27,7 @@ function Foo() {} ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; -/** +import { c as _c } from "react/compiler-runtime"; /** * Should produce 3 scopes: * * a: inputs=props.a, outputs=a diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-function-expression-React-memo-gating.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-function-expression-React-memo-gating.expect.md index d513bffd4f..91d4fc15fb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-function-expression-React-memo-gating.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-function-expression-React-memo-gating.expect.md @@ -31,7 +31,7 @@ export default React.forwardRef( } : function notNamedLikeAComponent(props) { return
; - } + }, ); ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/interdependent.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/interdependent.expect.md index 243331bfe8..32e87ab33f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/interdependent.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/interdependent.expect.md @@ -27,8 +27,7 @@ function Foo() {} ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; -/** +import { c as _c } from "react/compiler-runtime"; /** * Should produce 1 scope: * * return: inputs=props.a & props.b; outputs=return diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-independently-memoizable-arg.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-independently-memoizable-arg.expect.md index b47275f203..2f93ccd275 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-independently-memoizable-arg.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-independently-memoizable-arg.expect.md @@ -31,7 +31,7 @@ function Component(props) { t0 = x?.(
{props.text} -
+
, ); $[0] = props; $[1] = t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.expect.md index bc275604f7..9a9314db7a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.expect.md @@ -29,7 +29,7 @@ function Component(props) { const x = makeObject(); const user = useFragment( graphql`fragment Component_user on User { ... }`, - props.user + props.user, ); const posts = user.timeline.posts.edges.nodes.map((node) => { x.y = true; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md index 7c6e440be4..3418d008c3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md @@ -26,7 +26,7 @@ function Component(props) { const $ = _c(5); const user = useFragment( graphql`fragment Component_user on User { ... }`, - props.user + props.user, ); let posts; if ($[0] !== user.timeline.posts.edges.nodes) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-no-value-for-temporary-reactive-scope-with-early-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-no-value-for-temporary-reactive-scope-with-early-return.expect.md index e75ea02545..48a29a8f8c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-no-value-for-temporary-reactive-scope-with-early-return.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-no-value-for-temporary-reactive-scope-with-early-return.expect.md @@ -56,7 +56,7 @@ function Component(props) { {fbt._( "Lorum ipsum{thing} blah blah blah", [fbt._param("thing", object.b)], - { hk: "lwmuH" } + { hk: "lwmuH" }, )}
); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-unmerged-fbt-call-merge-overlapping-reactive-scopes.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-unmerged-fbt-call-merge-overlapping-reactive-scopes.expect.md index da1dcfb742..845f8488fa 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-unmerged-fbt-call-merge-overlapping-reactive-scopes.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-unmerged-fbt-call-merge-overlapping-reactive-scopes.expect.md @@ -42,7 +42,7 @@ function Component(props) { const label = fbt._( { "*": "{number} bars", _1: "1 bar" }, [fbt._plural(props.value.length, "number")], - { hk: "4mUen7" } + { hk: "4mUen7" }, ); t0 = props.cond ? ( diff --git a/compiler/packages/make-read-only-util/src/makeReadOnly.ts b/compiler/packages/make-read-only-util/src/makeReadOnly.ts index a9df3bacdb..4c5279eccd 100644 --- a/compiler/packages/make-read-only-util/src/makeReadOnly.ts +++ b/compiler/packages/make-read-only-util/src/makeReadOnly.ts @@ -127,7 +127,11 @@ function buildMakeReadOnly( Object.getOwnPropertyDescriptors(o) )) { if (!cache.has(k) && isWriteable(prop)) { - if (prop.hasOwnProperty("set") || prop.hasOwnProperty("get") || k === "current") { + if ( + prop.hasOwnProperty("set") || + prop.hasOwnProperty("get") || + k === "current" + ) { // - we currently don't handle accessor properties // - we currently have no other way of checking whether an object // is a `ref` (i.e. returned by useRef). diff --git a/compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts b/compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts index 1cf668eae1..1b6494ed92 100644 --- a/compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts +++ b/compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts @@ -16,7 +16,7 @@ import BabelPluginReactCompiler, { import { LoggerEvent as RawLoggerEvent } from "babel-plugin-react-compiler/src/Entrypoint"; import chalk from "chalk"; -type LoggerEvent = RawLoggerEvent & {filename: string | null}; +type LoggerEvent = RawLoggerEvent & { filename: string | null }; const SucessfulCompilation: Array = []; const ActionableFailures: Array = []; @@ -24,7 +24,7 @@ const OtherFailures: Array = []; const logger = { logEvent(filename: string | null, rawEvent: RawLoggerEvent) { - const event = {...rawEvent, filename}; + const event = { ...rawEvent, filename }; switch (event.kind) { case "CompileSuccess": { SucessfulCompilation.push(event); @@ -140,8 +140,8 @@ export default { report(): void { const totalComponents = SucessfulCompilation.length + - countUniqueLocInEvents(OtherFailures) + - countUniqueLocInEvents(ActionableFailures) + countUniqueLocInEvents(OtherFailures) + + countUniqueLocInEvents(ActionableFailures); console.log( chalk.green( `Successfully compiled ${SucessfulCompilation.length} out of ${totalComponents} components.` diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 6c947273b1..e7eb5f88f6 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -271,7 +271,10 @@ function getEvaluatorPresets( ); return presets; } -async function format(inputCode: string, language: "typescript" | "flow"): Promise { +async function format( + inputCode: string, + language: "typescript" | "flow" +): Promise { return await prettier.format(inputCode, { semi: true, parser: language === "typescript" ? "babel-ts" : "flow", @@ -294,7 +297,9 @@ export async function transformFixtureInput( parseConfigPragmaFn: typeof ParseConfigPragma, plugin: BabelCore.PluginObj, includeEvaluator: boolean -): Promise<{ kind: "ok"; value: TransformResult } | { kind: "err"; msg: string }> { +): Promise< + { kind: "ok"; value: TransformResult } | { kind: "err"; msg: string } +> { // Extract the first line to quickly check for custom test directives const firstLine = input.substring(0, input.indexOf("\n")); diff --git a/compiler/scripts/release/publish-manual.js b/compiler/scripts/release/publish-manual.js index bb40c63b7c..203510983e 100644 --- a/compiler/scripts/release/publish-manual.js +++ b/compiler/scripts/release/publish-manual.js @@ -25,7 +25,7 @@ const spawnHelper = util.promisify(_spawn); function execHelper(command, options, streamStdout = false) { return new Promise((resolve, reject) => { const proc = cp.exec(command, options, (error, stdout) => - error ? reject(error) : resolve(stdout.trim()), + error ? reject(error) : resolve(stdout.trim()) ); if (streamStdout) { proc.stdout.pipe(process.stdout); @@ -39,7 +39,7 @@ function sleep(ms) { async function getDateStringForCommit(commit) { let dateString = await execHelper( - `git show -s --no-show-signature --format=%cd --date=format:%Y%m%d ${commit}`, + `git show -s --no-show-signature --format=%cd --date=format:%Y%m%d ${commit}` ); // On CI environment, this string is wrapped with quotes '...'s @@ -99,7 +99,7 @@ async function main() { const isPristine = (await execHelper("git status --porcelain")) === ""; if (currBranchName !== "main" || isPristine === false) { throw new Error( - "This script must be run from the `main` branch with no uncommitted changes", + "This script must be run from the `main` branch with no uncommitted changes" ); } } @@ -111,7 +111,7 @@ async function main() { const spinner = ora( `Preparing to publish ${ forReal === true ? "(for real)" : "(dry run)" - } [debug=${debug}]`, + } [debug=${debug}]` ).info(); spinner.info("Building packages"); @@ -145,7 +145,7 @@ async function main() { spinner.stop(`Successfully packed ${pkgName} (dry run)`); } spinner.succeed( - "Please confirm contents of packages before publishing. You can run this command again with --for-real to publish to npm", + "Please confirm contents of packages before publishing. You can run this command again with --for-real to publish to npm" ); } @@ -155,7 +155,7 @@ async function main() { "git show -s --no-show-signature --format=%h", { cwd: path.resolve(__dirname, ".."), - }, + } ); const dateString = await getDateStringForCommit(commit); @@ -175,20 +175,20 @@ async function main() { `yarn version --new-version ${newVersion} --no-git-tag-version`, { cwd: pkgDir, - }, + } ); await execHelper( `git add package.json && git commit -m "Bump version to ${newVersion}"`, { cwd: pkgDir, - }, + } ); } catch (e) { spinner.fail(e.toString()); throw e; } spinner.succeed( - `Bumped ${pkgName} to ${newVersion} and added a git commit`, + `Bumped ${pkgName} to ${newVersion} and added a git commit` ); } @@ -196,7 +196,7 @@ async function main() { spinner.info( `🚨🚨🚨 About to publish to npm in ${ TIME_TO_RECONSIDER / 1000 - } seconds. You still have time to kill this script!`, + } seconds. You still have time to kill this script!` ); await sleep(TIME_TO_RECONSIDER); } @@ -214,7 +214,7 @@ async function main() { { cwd: pkgDir, stdio: "inherit", - }, + } ); console.log("\n"); } catch (e) { From 9d530e94c40a9adf3fbf24390ee09f10dc5dcc2d Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 29 May 2024 11:41:01 +0900 Subject: [PATCH 14/26] [compiler:babel] Don't read config files when not running as part of user's pipeline When the user app has a babel.config file that is missing the compiler, strange things happen as babel does some strange merging of options from the user's config and in various callsites like in our eslint rule and healthcheck script. To minimize odd behavior, we default to not reading the user's babel.config Fixes #29135 ghstack-source-id: d6fdc43c5c9107645f36718203873aa3f6228475 Pull Request resolved: https://github.com/facebook/react/pull/29211 --- .../src/Babel/RunReactCompilerBabelPlugin.ts | 2 ++ .../src/rules/ReactCompilerRule.ts | 2 ++ .../react-compiler-healthcheck/src/checks/reactCompiler.ts | 2 ++ compiler/packages/snap/src/compiler.ts | 6 ++++++ 4 files changed, 12 insertions(+) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Babel/RunReactCompilerBabelPlugin.ts b/compiler/packages/babel-plugin-react-compiler/src/Babel/RunReactCompilerBabelPlugin.ts index 7d0248300c..8218cf267b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Babel/RunReactCompilerBabelPlugin.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Babel/RunReactCompilerBabelPlugin.ts @@ -36,6 +36,8 @@ export function runBabelPluginReactCompiler( "babel-plugin-fbt-runtime", ], sourceType: "module", + configFile: false, + babelrc: false, }); invariant( result?.code != null, diff --git a/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts b/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts index 3c16941e75..fd33ae0339 100644 --- a/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts +++ b/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts @@ -143,6 +143,8 @@ const rule: Rule.RuleModule = { [BabelPluginReactCompiler, options], ], sourceType: "module", + configFile: false, + babelrc: false, }); } catch (err) { if (isReactCompilerError(err) && Array.isArray(err.details)) { diff --git a/compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts b/compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts index 1b6494ed92..09c9b9bda6 100644 --- a/compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts +++ b/compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts @@ -85,6 +85,8 @@ function runBabelPluginReactCompiler( retainLines: true, plugins: [[BabelPluginReactCompiler, options]], sourceType: "module", + configFile: false, + babelrc: false, }); if (result?.code == null) { throw new Error( diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index e7eb5f88f6..ab2cf5cef8 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -333,6 +333,8 @@ export async function transformFixtureInput( sourceType: "module", ast: includeEvaluator, cloneInputAst: includeEvaluator, + configFile: false, + babelrc: false, }); invariant( forgetResult?.code != null, @@ -355,6 +357,8 @@ export async function transformFixtureInput( const result = transformFromAstSync(forgetResult.ast, forgetOutput, { presets, filename: virtualFilepath, + configFile: false, + babelrc: false, }); if (result?.code == null) { return { @@ -379,6 +383,8 @@ export async function transformFixtureInput( const result = transformFromAstSync(inputAst, input, { presets, filename: virtualFilepath, + configFile: false, + babelrc: false, }); if (result?.code == null) { From 81c3775816478c613b05689dfa3f481827a84249 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 29 May 2024 11:47:42 +0900 Subject: [PATCH 15/26] [compiler] Ignore run prettier commit in git blame --- compiler/.git-blame-ignore-revs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/.git-blame-ignore-revs b/compiler/.git-blame-ignore-revs index ee5f58bca6..0b968ae684 100644 --- a/compiler/.git-blame-ignore-revs +++ b/compiler/.git-blame-ignore-revs @@ -1,2 +1,3 @@ 741ae6e3aa10735722a442a3c8be77af7c951204 -59cba458af27d936df8feabf641f545f431529ad \ No newline at end of file +59cba458af27d936df8feabf641f545f431529ad +c998bb1ed4b3285398c9c7797135d3f060243c6a From 84c47b3d527512246a42e312f3328d0b0d18829a Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 29 May 2024 12:01:35 +0900 Subject: [PATCH 16/26] Bump version to 0.0.0-experimental-487cb0e-20240529 --- compiler/packages/babel-plugin-react-compiler/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/packages/babel-plugin-react-compiler/package.json b/compiler/packages/babel-plugin-react-compiler/package.json index 4d1d9f35a7..b53d374e28 100644 --- a/compiler/packages/babel-plugin-react-compiler/package.json +++ b/compiler/packages/babel-plugin-react-compiler/package.json @@ -1,6 +1,6 @@ { "name": "babel-plugin-react-compiler", - "version": "0.0.0-experimental-592953e-20240517", + "version": "0.0.0-experimental-487cb0e-20240529", "description": "Babel plugin for React Compiler.", "main": "dist/index.js", "license": "MIT", From bd30dc3ae23acbb42b29b68fcb91c339e6f7c829 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 29 May 2024 12:01:35 +0900 Subject: [PATCH 17/26] Bump version to 0.0.0-experimental-a97cca1-20240529 --- compiler/packages/eslint-plugin-react-compiler/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/packages/eslint-plugin-react-compiler/package.json b/compiler/packages/eslint-plugin-react-compiler/package.json index fa48159859..25bfc0dcb1 100644 --- a/compiler/packages/eslint-plugin-react-compiler/package.json +++ b/compiler/packages/eslint-plugin-react-compiler/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-react-compiler", - "version": "0.0.0-experimental-c8b3f72-20240517", + "version": "0.0.0-experimental-a97cca1-20240529", "description": "ESLint plugin to display errors found by the React compiler.", "main": "dist/index.js", "scripts": { From b44263addb4c4f74cb6520c5f652b35bda24a015 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 29 May 2024 12:01:36 +0900 Subject: [PATCH 18/26] Bump version to 0.0.0-experimental-31393f7-20240529 --- compiler/packages/react-compiler-healthcheck/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/packages/react-compiler-healthcheck/package.json b/compiler/packages/react-compiler-healthcheck/package.json index a2c74b3a69..8839e62efe 100644 --- a/compiler/packages/react-compiler-healthcheck/package.json +++ b/compiler/packages/react-compiler-healthcheck/package.json @@ -1,6 +1,6 @@ { "name": "react-compiler-healthcheck", - "version": "0.0.0-experimental-f978439-20240517", + "version": "0.0.0-experimental-31393f7-20240529", "description": "Health check script to test violations of the rules of react.", "bin": { "react-compiler-healthcheck": "dist/index.js" From 3b29ed16386c1afb2e76c3db0d576184154ec141 Mon Sep 17 00:00:00 2001 From: Timothy Yung Date: Tue, 28 May 2024 20:36:41 -0700 Subject: [PATCH 19/26] Fix "findNodeHandle inside its render()" False Positive Warning (#29627) This was missed in https://github.com/facebook/react/pull/29038 when unifying the "owner" abstractions, causing `findNodeHandle` to warn even outside of `render()` invocations. --- packages/react-native-renderer/src/ReactNativePublicCompat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native-renderer/src/ReactNativePublicCompat.js b/packages/react-native-renderer/src/ReactNativePublicCompat.js index 7d6bb49741..c50b881c15 100644 --- a/packages/react-native-renderer/src/ReactNativePublicCompat.js +++ b/packages/react-native-renderer/src/ReactNativePublicCompat.js @@ -90,7 +90,7 @@ export function findHostInstance_DEPRECATED( export function findNodeHandle(componentOrHandle: any): ?number { if (__DEV__) { const owner = currentOwner; - if (owner !== null && owner.stateNode !== null) { + if (owner !== null && isRendering && owner.stateNode !== null) { if (!owner.stateNode._warnedAboutRefsInRender) { console.error( '%s is accessing findNodeHandle inside its render(). ' + From e2e12f33517e528bc955fe3f0098d6765f4648af Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 29 May 2024 11:55:11 +0900 Subject: [PATCH 20/26] Update .git-blame-ignore-revs - Moves the file as it needs to be in root git directory - Removes now unreachable commits due to repo merge - Add run prettier commit c998bb1ed4b3285398c9c7797135d3f060243c6a to ignored revs ghstack-source-id: d9dfa7099fbc7782fbce600af4caafd405c196cb Pull Request resolved: https://github.com/facebook/react/pull/29630 --- .git-blame-ignore-revs | 1 + compiler/.git-blame-ignore-revs | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) create mode 100644 .git-blame-ignore-revs delete mode 100644 compiler/.git-blame-ignore-revs diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 0000000000..63d0d11fb6 --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1 @@ +c998bb1ed4b3285398c9c7797135d3f060243c6a diff --git a/compiler/.git-blame-ignore-revs b/compiler/.git-blame-ignore-revs deleted file mode 100644 index 0b968ae684..0000000000 --- a/compiler/.git-blame-ignore-revs +++ /dev/null @@ -1,3 +0,0 @@ -741ae6e3aa10735722a442a3c8be77af7c951204 -59cba458af27d936df8feabf641f545f431529ad -c998bb1ed4b3285398c9c7797135d3f060243c6a From 49ed6f0740f9d47777faafd92046f7f044cf3e5e Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 25 May 2024 22:03:52 +0100 Subject: [PATCH 21/26] compiler: Allow global mutation in jsx props Fixes https://x.com/raibima/status/1794395807216738792 The issue is that if you pass a global-modifying function as prop to JSX, we currently report that it's invalid to modify a global during rendering. The problem is that we don't really know when/if the child component will actually call that function prop. It would be against the rules to call the function during render, but it's totally fine to call it during an event handler or from a useEffect. Since we don't know at the call-site how the child will use the function, we should allow such calls. In the future we could improve this in a few ways: * For all functions that modify globals, codegen an assertion or warning into the function that fires if it's called "during render". We'd have to precisely define what "during render" is, but this would at least help developers catch this dynamically. * Use the type system to distinguish "event/effect" and "render" functions to help developers avoid accidentally mutating globals during render. ghstack-source-id: 4aba4e6d214fd6c062e4029294efe9b8fe25cd83 Pull Request resolved: https://github.com/facebook/react/pull/29591 --- .../src/Inference/InferReferenceEffects.ts | 51 ++++++++++- ...ow-modify-global-in-callback-jsx.expect.md | 86 +++++++++++++++++++ .../allow-modify-global-in-callback-jsx.js | 24 ++++++ ...ment-to-global-function-jsx-prop.expect.md | 53 ++++++++++++ ...eassignment-to-global-function-jsx-prop.js | 16 ++++ ...global-in-component-tag-function.expect.md | 27 ++++++ ...assign-global-in-component-tag-function.js | 6 ++ ...or.assign-global-in-jsx-children.expect.md | 30 +++++++ .../error.assign-global-in-jsx-children.js | 9 ++ ...n-global-in-jsx-spread-attribute.expect.md | 27 ++++++ ...r.assign-global-in-jsx-spread-attribute.js | 6 ++ ...signment-to-global-function-prop.expect.md | 32 ------- ...or.reassignment-to-global-function-prop.js | 11 --- 13 files changed, 331 insertions(+), 47 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index 3724c8d6f1..520684c026 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -1103,13 +1103,56 @@ function inferBlock( break; } case "JsxExpression": { - valueKind = { + if (instrValue.tag.kind === "Identifier") { + state.referenceAndRecordEffects( + instrValue.tag, + Effect.Freeze, + ValueReason.JsxCaptured, + functionEffects + ); + } + if (instrValue.children !== null) { + for (const child of instrValue.children) { + state.referenceAndRecordEffects( + child, + Effect.Freeze, + ValueReason.JsxCaptured, + functionEffects + ); + } + } + for (const attr of instrValue.props) { + if (attr.kind === "JsxSpreadAttribute") { + state.referenceAndRecordEffects( + attr.argument, + Effect.Freeze, + ValueReason.JsxCaptured, + functionEffects + ); + } else { + const propEffects: Array = []; + state.referenceAndRecordEffects( + attr.place, + Effect.Freeze, + ValueReason.JsxCaptured, + propEffects + ); + functionEffects.push( + ...propEffects.filter( + (propEffect) => propEffect.kind !== "GlobalMutation" + ) + ); + } + } + + state.initialize(instrValue, { kind: ValueKind.Frozen, reason: new Set([ValueReason.Other]), context: new Set(), - }; - effect = { kind: Effect.Freeze, reason: ValueReason.JsxCaptured }; - break; + }); + state.define(instr.lvalue, instrValue); + instr.lvalue.effect = Effect.ConditionallyMutate; + continue; } case "JsxFragment": { valueKind = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md new file mode 100644 index 0000000000..43ab697f87 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md @@ -0,0 +1,86 @@ + +## Input + +```javascript +import { useMemo } from "react"; + +const someGlobal = { value: 0 }; + +function Component({ value }) { + const onClick = () => { + someGlobal.value = value; + }; + return useMemo(() => { + return
{someGlobal.value}
; + }, []); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 0 }], + sequentialRenders: [ + { value: 1 }, + { value: 1 }, + { value: 42 }, + { value: 42 }, + { value: 0 }, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useMemo } from "react"; + +const someGlobal = { value: 0 }; + +function Component(t0) { + const $ = _c(4); + const { value } = t0; + let t1; + if ($[0] !== value) { + t1 = () => { + someGlobal.value = value; + }; + $[0] = value; + $[1] = t1; + } else { + t1 = $[1]; + } + const onClick = t1; + let t2; + let t3; + if ($[2] !== onClick) { + t3 =
{someGlobal.value}
; + $[2] = onClick; + $[3] = t3; + } else { + t3 = $[3]; + } + t2 = t3; + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 0 }], + sequentialRenders: [ + { value: 1 }, + { value: 1 }, + { value: 42 }, + { value: 42 }, + { value: 0 }, + ], +}; + +``` + +### Eval output +(kind: ok)
0
+
0
+
0
+
0
+
0
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js new file mode 100644 index 0000000000..7404b4b6c0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js @@ -0,0 +1,24 @@ +import { useMemo } from "react"; + +const someGlobal = { value: 0 }; + +function Component({ value }) { + const onClick = () => { + someGlobal.value = value; + }; + return useMemo(() => { + return
{someGlobal.value}
; + }, []); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 0 }], + sequentialRenders: [ + { value: 1 }, + { value: 1 }, + { value: 42 }, + { value: 42 }, + { value: 0 }, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.expect.md new file mode 100644 index 0000000000..eaa2834a49 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.expect.md @@ -0,0 +1,53 @@ + +## Input + +```javascript +function Component() { + const onClick = () => { + // Cannot assign to globals + someUnknownGlobal = true; + moduleLocal = true; + }; + // It's possible that this could be an event handler / effect function, + // but we don't know that and optimistically assume it will only be + // called by an event handler or effect, where it is allowed to modify globals + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const onClick = () => { + someUnknownGlobal = true; + moduleLocal = true; + }; + + t0 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.js new file mode 100644 index 0000000000..8b9da9eb31 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.js @@ -0,0 +1,16 @@ +function Component() { + const onClick = () => { + // Cannot assign to globals + someUnknownGlobal = true; + moduleLocal = true; + }; + // It's possible that this could be an event handler / effect function, + // but we don't know that and optimistically assume it will only be + // called by an event handler or effect, where it is allowed to modify globals + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.expect.md new file mode 100644 index 0000000000..5553f235a0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +function Component() { + const Foo = () => { + someGlobal = true; + }; + return ; +} + +``` + + +## Error + +``` + 1 | function Component() { + 2 | const Foo = () => { +> 3 | someGlobal = true; + | ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3) + 4 | }; + 5 | return ; + 6 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.js new file mode 100644 index 0000000000..2982fdf708 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.js @@ -0,0 +1,6 @@ +function Component() { + const Foo = () => { + someGlobal = true; + }; + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.expect.md new file mode 100644 index 0000000000..d380137836 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.expect.md @@ -0,0 +1,30 @@ + +## Input + +```javascript +function Component() { + const foo = () => { + someGlobal = true; + }; + // Children are generally access/called during render, so + // modifying a global in a children function is almost + // certainly a mistake. + return {foo}; +} + +``` + + +## Error + +``` + 1 | function Component() { + 2 | const foo = () => { +> 3 | someGlobal = true; + | ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3) + 4 | }; + 5 | // Children are generally access/called during render, so + 6 | // modifying a global in a children function is almost +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.js new file mode 100644 index 0000000000..82554e8ac4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.js @@ -0,0 +1,9 @@ +function Component() { + const foo = () => { + someGlobal = true; + }; + // Children are generally access/called during render, so + // modifying a global in a children function is almost + // certainly a mistake. + return {foo}; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md new file mode 100644 index 0000000000..3861b16e90 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +function Component() { + const foo = () => { + someGlobal = true; + }; + return
; +} + +``` + + +## Error + +``` + 1 | function Component() { + 2 | const foo = () => { +> 3 | someGlobal = true; + | ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3) + 4 | }; + 5 | return
; + 6 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js new file mode 100644 index 0000000000..1eea9267b5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js @@ -0,0 +1,6 @@ +function Component() { + const foo = () => { + someGlobal = true; + }; + return
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.expect.md deleted file mode 100644 index 56132c34c1..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.expect.md +++ /dev/null @@ -1,32 +0,0 @@ - -## Input - -```javascript -function Component() { - const foo = () => { - // Cannot assign to globals - someUnknownGlobal = true; - moduleLocal = true; - }; - // It's possible that this could be an event handler / effect function, - // but we don't know that and conservatively assume it's a render helper - // where it's disallowed to modify globals - return ; -} - -``` - - -## Error - -``` - 2 | const foo = () => { - 3 | // Cannot assign to globals -> 4 | someUnknownGlobal = true; - | ^^^^^^^^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (4:4) - 5 | moduleLocal = true; - 6 | }; - 7 | // It's possible that this could be an event handler / effect function, -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.js deleted file mode 100644 index 926f4c048f..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.js +++ /dev/null @@ -1,11 +0,0 @@ -function Component() { - const foo = () => { - // Cannot assign to globals - someUnknownGlobal = true; - moduleLocal = true; - }; - // It's possible that this could be an event handler / effect function, - // but we don't know that and conservatively assume it's a render helper - // where it's disallowed to modify globals - return ; -} From c272789ce54ab2db3fb1af53c54631a7246b31bc Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 25 May 2024 22:24:49 +0100 Subject: [PATCH 22/26] compiler: Add todo for getter/setter syntax We were missing a check that ObjectMethods are not getters or setters. In our experience this is pretty rare within React components and hooks themselves, so let's start with a todo. Closes #29586 ghstack-source-id: 03c6cce9a9368a4a4f4ba98bcdff3fa4729ceaf9 Pull Request resolved: https://github.com/facebook/react/pull/29592 --- .../src/HIR/BuildHIR.ts | 9 ++++ ...odo-object-expression-get-syntax.expect.md | 39 ++++++++++++++++++ ...error.todo-object-expression-get-syntax.js | 14 +++++++ ...odo-object-expression-set-syntax.expect.md | 41 +++++++++++++++++++ ...error.todo-object-expression-set-syntax.js | 16 ++++++++ 5 files changed, 119 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-get-syntax.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-get-syntax.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-set-syntax.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-set-syntax.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index 463881d2c4..59e2f0c89f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -1520,6 +1520,15 @@ function lowerExpression( place, }); } else if (propertyPath.isObjectMethod()) { + if (propertyPath.node.kind !== "method") { + builder.errors.push({ + reason: `(BuildHIR::lowerExpression) Handle ${propertyPath.node.kind} functions in ObjectExpression`, + severity: ErrorSeverity.Todo, + loc: propertyPath.node.loc ?? null, + suggestions: null, + }); + continue; + } const method = lowerObjectMethod(builder, propertyPath); const place = lowerValueToTemporary(builder, method); const loweredKey = lowerObjectPropertyKey(builder, propertyPath); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-get-syntax.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-get-syntax.expect.md new file mode 100644 index 0000000000..d713d07fd2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-get-syntax.expect.md @@ -0,0 +1,39 @@ + +## Input + +```javascript +function Component({ value }) { + const object = { + get value() { + return value; + }, + }; + return
{object.value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [{ value: 0 }], + sequentialRenders: [{ value: 1 }, { value: 2 }], +}; + +``` + + +## Error + +``` + 1 | function Component({ value }) { + 2 | const object = { +> 3 | get value() { + | ^^^^^^^^^^^^^ +> 4 | return value; + | ^^^^^^^^^^^^^^^^^^^ +> 5 | }, + | ^^^^^^ Todo: (BuildHIR::lowerExpression) Handle get functions in ObjectExpression (3:5) + 6 | }; + 7 | return
{object.value}
; + 8 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-get-syntax.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-get-syntax.js new file mode 100644 index 0000000000..e96134edf1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-get-syntax.js @@ -0,0 +1,14 @@ +function Component({ value }) { + const object = { + get value() { + return value; + }, + }; + return
{object.value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [{ value: 0 }], + sequentialRenders: [{ value: 1 }, { value: 2 }], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-set-syntax.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-set-syntax.expect.md new file mode 100644 index 0000000000..1d917b4d3a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-set-syntax.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +function Component(props) { + let value; + const object = { + set value(v) { + value = v; + }, + }; + object.value = props.value; + return
{value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [{ value: 0 }], + sequentialRenders: [{ value: 1 }, { value: 2 }], +}; + +``` + + +## Error + +``` + 2 | let value; + 3 | const object = { +> 4 | set value(v) { + | ^^^^^^^^^^^^^^ +> 5 | value = v; + | ^^^^^^^^^^^^^^^^ +> 6 | }, + | ^^^^^^ Todo: (BuildHIR::lowerExpression) Handle set functions in ObjectExpression (4:6) + 7 | }; + 8 | object.value = props.value; + 9 | return
{value}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-set-syntax.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-set-syntax.js new file mode 100644 index 0000000000..5030bea6fc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-object-expression-set-syntax.js @@ -0,0 +1,16 @@ +function Component(props) { + let value; + const object = { + set value(v) { + value = v; + }, + }; + object.value = props.value; + return
{value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [{ value: 0 }], + sequentialRenders: [{ value: 1 }, { value: 2 }], +}; From afb2c39ec36d40fff362be465e2a310661469630 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 20 May 2024 18:20:04 +0100 Subject: [PATCH 23/26] compiler: fixtures for fast-refresh mode (w todos) ghstack-source-id: 65dd14fe9b37328bd60fe791b23dde54da10b285 Pull Request resolved: https://github.com/facebook/react/pull/29175 --- .../ReactiveScopes/CodegenReactiveFunction.ts | 19 +-- ...-dont-refresh-const-changes-prod.expect.md | 104 ++++++++++++++++ ...refresh-dont-refresh-const-changes-prod.js | 35 ++++++ ...esh-refresh-on-const-changes-dev.expect.md | 112 ++++++++++++++++++ ...st-refresh-refresh-on-const-changes-dev.js | 38 ++++++ ...ct.md => fast-refresh-reloading.expect.md} | 0 ...reloading.js => fast-refresh-reloading.js} | 0 .../packages/snap/src/SproutTodoFilter.ts | 2 + 8 files changed, 302 insertions(+), 8 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-dont-refresh-const-changes-prod.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-dont-refresh-const-changes-prod.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-refresh-on-const-changes-dev.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-refresh-on-const-changes-dev.js rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{hot-module-reloading.expect.md => fast-refresh-reloading.expect.md} (100%) rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{hot-module-reloading.js => fast-refresh-reloading.js} (100%) 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 2c0364ed12..075fb98792 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -81,18 +81,21 @@ export function codegenFunction( ); /** - * Hot-module reloading reuses component instances at runtime even as the source of the component changes. + * Fast Refresh reuses component instances at runtime even as the source of the component changes. * The generated code needs to prevent values from one version of the code being reused after a code cange. * If HMR detection is enabled and we know the source code of the component, assign a cache slot to track * the source hash, and later, emit code to check for source changes and reset the cache on source changes. */ - let hotModuleReloadState: { cacheIndex: number; hash: string } | null = null; + let fastRefreshState: { + cacheIndex: number; + hash: string; + } | null = null; if ( fn.env.config.enableResetCacheOnSourceFileChanges && fn.env.code !== null ) { const hash = createHmac("sha256", fn.env.code).digest("hex"); - hotModuleReloadState = { + fastRefreshState = { cacheIndex: cx.nextCacheIndex, hash, }; @@ -131,7 +134,7 @@ export function codegenFunction( ), ]) ); - if (hotModuleReloadState !== null) { + if (fastRefreshState !== null) { // HMR detection is enabled, emit code to reset the memo cache on source changes const index = cx.synthesizeName("$i"); preface.push( @@ -140,10 +143,10 @@ export function codegenFunction( "!==", t.memberExpression( t.identifier(cx.synthesizeName("$")), - t.numericLiteral(hotModuleReloadState.cacheIndex), + t.numericLiteral(fastRefreshState.cacheIndex), true ), - t.stringLiteral(hotModuleReloadState.hash) + t.stringLiteral(fastRefreshState.hash) ), t.blockStatement([ t.forStatement( @@ -185,10 +188,10 @@ export function codegenFunction( "=", t.memberExpression( t.identifier(cx.synthesizeName("$")), - t.numericLiteral(hotModuleReloadState.cacheIndex), + t.numericLiteral(fastRefreshState.cacheIndex), true ), - t.stringLiteral(hotModuleReloadState.hash) + t.stringLiteral(fastRefreshState.hash) ) ), ]) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-dont-refresh-const-changes-prod.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-dont-refresh-const-changes-prod.expect.md new file mode 100644 index 0000000000..87084a1402 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-dont-refresh-const-changes-prod.expect.md @@ -0,0 +1,104 @@ + +## Input + +```javascript +// @compilationMode(infer) +import { useEffect, useMemo, useState } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +let pretendConst = 0; + +function unsafeResetConst() { + pretendConst = 0; +} + +function unsafeUpdateConst() { + pretendConst += 1; +} + +function Component() { + useState(() => { + // unsafe: reset the constant when first rendering the instance + unsafeResetConst(); + }); + // UNSAFE! changing a module variable that is read by a component is normally + // unsafe, but in this case we're simulating a fast refresh between each render + unsafeUpdateConst(); + + // In production mode (no @enableResetCacheOnSourceFileChanges) memo caches are not + // reset unless the deps change + const value = useMemo(() => [{ pretendConst }], []); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @compilationMode(infer) +import { useEffect, useMemo, useState } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +let pretendConst = 0; + +function unsafeResetConst() { + pretendConst = 0; +} + +function unsafeUpdateConst() { + pretendConst += 1; +} + +function Component() { + const $ = _c(3); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = () => { + unsafeResetConst(); + }; + $[0] = t0; + } else { + t0 = $[0]; + } + useState(t0); + + unsafeUpdateConst(); + let t1; + let t2; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t2 = [{ pretendConst }]; + $[1] = t2; + } else { + t2 = $[1]; + } + t1 = t2; + const value = t1; + let t3; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t3 = ; + $[2] = t3; + } else { + t3 = $[2]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}], +}; + +``` + +### Eval output +(kind: ok)
{"inputs":[],"output":[{"pretendConst":1}]}
+
{"inputs":[],"output":[{"pretendConst":1}]}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-dont-refresh-const-changes-prod.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-dont-refresh-const-changes-prod.js new file mode 100644 index 0000000000..6c4ed19e78 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-dont-refresh-const-changes-prod.js @@ -0,0 +1,35 @@ +// @compilationMode(infer) +import { useEffect, useMemo, useState } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +let pretendConst = 0; + +function unsafeResetConst() { + pretendConst = 0; +} + +function unsafeUpdateConst() { + pretendConst += 1; +} + +function Component() { + useState(() => { + // unsafe: reset the constant when first rendering the instance + unsafeResetConst(); + }); + // UNSAFE! changing a module variable that is read by a component is normally + // unsafe, but in this case we're simulating a fast refresh between each render + unsafeUpdateConst(); + + // In production mode (no @enableResetCacheOnSourceFileChanges) memo caches are not + // reset unless the deps change + const value = useMemo(() => [{ pretendConst }], []); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-refresh-on-const-changes-dev.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-refresh-on-const-changes-dev.expect.md new file mode 100644 index 0000000000..19317de47e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-refresh-on-const-changes-dev.expect.md @@ -0,0 +1,112 @@ + +## Input + +```javascript +// @compilationMode(infer) @enableResetCacheOnSourceFileChanges +import { useEffect, useMemo, useState } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +let pretendConst = 0; + +function unsafeResetConst() { + pretendConst = 0; +} + +function unsafeUpdateConst() { + pretendConst += 1; +} + +function Component() { + useState(() => { + // unsafe: reset the constant when first rendering the instance + unsafeResetConst(); + }); + // UNSAFE! changing a module variable that is read by a component is normally + // unsafe, but in this case we're simulating a fast refresh between each render + unsafeUpdateConst(); + + // TODO: In fast refresh mode (@enableResetCacheOnSourceFileChanges) Forget should + // reset on changes to globals that impact the component/hook, effectively memoizing + // as if value was reactive. However, we don't want to actually treat globals as + // reactive (though that would be trivial) since it could change compilation too much + // btw dev and prod. Instead, we should reset the cache via a secondary mechanism. + const value = useMemo(() => [{ pretendConst }], [pretendConst]); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @compilationMode(infer) @enableResetCacheOnSourceFileChanges +import { useEffect, useMemo, useState } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +let pretendConst = 0; + +function unsafeResetConst() { + pretendConst = 0; +} + +function unsafeUpdateConst() { + pretendConst += 1; +} + +function Component() { + const $ = _c(4); + if ( + $[0] !== "4bf230b116dd95f382060ad17350e116395e41ed757e51fd074ea0b4ed281272" + ) { + for (let $i = 0; $i < 4; $i += 1) { + $[$i] = Symbol.for("react.memo_cache_sentinel"); + } + $[0] = "4bf230b116dd95f382060ad17350e116395e41ed757e51fd074ea0b4ed281272"; + } + let t0; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t0 = () => { + unsafeResetConst(); + }; + $[1] = t0; + } else { + t0 = $[1]; + } + useState(t0); + + unsafeUpdateConst(); + let t1; + let t2; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t2 = [{ pretendConst }]; + $[2] = t2; + } else { + t2 = $[2]; + } + t1 = t2; + const value = t1; + let t3; + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + t3 = ; + $[3] = t3; + } else { + t3 = $[3]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-refresh-on-const-changes-dev.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-refresh-on-const-changes-dev.js new file mode 100644 index 0000000000..413ff7d1f9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-refresh-on-const-changes-dev.js @@ -0,0 +1,38 @@ +// @compilationMode(infer) @enableResetCacheOnSourceFileChanges +import { useEffect, useMemo, useState } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +let pretendConst = 0; + +function unsafeResetConst() { + pretendConst = 0; +} + +function unsafeUpdateConst() { + pretendConst += 1; +} + +function Component() { + useState(() => { + // unsafe: reset the constant when first rendering the instance + unsafeResetConst(); + }); + // UNSAFE! changing a module variable that is read by a component is normally + // unsafe, but in this case we're simulating a fast refresh between each render + unsafeUpdateConst(); + + // TODO: In fast refresh mode (@enableResetCacheOnSourceFileChanges) Forget should + // reset on changes to globals that impact the component/hook, effectively memoizing + // as if value was reactive. However, we don't want to actually treat globals as + // reactive (though that would be trivial) since it could change compilation too much + // btw dev and prod. Instead, we should reset the cache via a secondary mechanism. + const value = useMemo(() => [{ pretendConst }], [pretendConst]); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hot-module-reloading.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-reloading.expect.md similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hot-module-reloading.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-reloading.expect.md diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hot-module-reloading.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-reloading.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hot-module-reloading.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fast-refresh-reloading.js diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index a8fc53606d..14ed51b2cc 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -493,6 +493,8 @@ const skipFilter = new Set([ // 'react-compiler-runtime' not yet supported "flag-enable-emit-hook-guards", + + "fast-refresh-refresh-on-const-changes-dev", ]); export default skipFilter; From 38e3b23483bf7a612391cd617a8926aa1f3cf52e Mon Sep 17 00:00:00 2001 From: Sophie Alpert Date: Wed, 29 May 2024 08:41:10 -0700 Subject: [PATCH 24/26] Tweak error message for "Should have a queue" (#29626) --- packages/react-reconciler/src/ReactFiberHooks.js | 6 ++++-- scripts/error-codes/codes.json | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index e66e12c515..f69b8f1a2f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1260,7 +1260,8 @@ function updateReducerImpl( if (queue === null) { throw new Error( - 'Should have a queue. This is likely a bug in React. Please file an issue.', + 'Should have a queue. You are likely calling Hooks conditionally, ' + + 'which is not allowed. (https://react.dev/link/invalid-hook-call)', ); } @@ -1506,7 +1507,8 @@ function rerenderReducer( if (queue === null) { throw new Error( - 'Should have a queue. This is likely a bug in React. Please file an issue.', + 'Should have a queue. You are likely calling Hooks conditionally, ' + + 'which is not allowed. (https://react.dev/link/invalid-hook-call)', ); } diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index c26ae4cce2..9bb82658ac 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -305,7 +305,7 @@ "308": "Context can only be read while React is rendering. In classes, you can read it in the render method or getDerivedStateFromProps. In function components, you can read it directly in the function body, but not inside Hooks like useReducer() or useMemo().", "309": "Function components cannot have string refs. We recommend using useRef() instead. Learn more about using refs safely here: https://react.dev/link/strict-mode-string-ref", "310": "Rendered more hooks than during the previous render.", - "311": "Should have a queue. This is likely a bug in React. Please file an issue.", + "311": "Should have a queue. You are likely calling Hooks conditionally, which is not allowed. (https://react.dev/link/invalid-hook-call)", "312": "Rendered more hooks than during the previous render", "313": "Unknown priority level. This error is likely caused by a bug in React. Please file an issue.", "314": "Pinged unknown suspense boundary type. This is probably a bug in React.", From 320da675705e8700bc1377a5fa22b4cdf1e52704 Mon Sep 17 00:00:00 2001 From: Niklas Mollenhauer Date: Wed, 29 May 2024 18:17:43 +0200 Subject: [PATCH 25/26] feat(compiler): Compiler Logical Negation Constant Propagation (#29623) ## Summary Resolves #29622 ## How did you test this change? I verified the implementation using the test. Note: This PR was done without waiting for approval in #29622, so feel free to just close it. --- .../src/Optimization/ConstantPropagation.ts | 19 ++++ .../constant-propagation-unary.expect.md | 86 +++++++++++++++++++ .../compiler/constant-propagation-unary.js | 35 ++++++++ 3 files changed, 140 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts index 04a401143a..e2116c9d94 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts @@ -311,6 +311,25 @@ function evaluateInstruction( } return null; } + case "UnaryExpression": { + switch (value.operator) { + case "!": { + const operand = read(constants, value.value); + if (operand !== null && operand.kind === "Primitive") { + const result: Primitive = { + kind: "Primitive", + value: !operand.value, + loc: value.loc, + }; + instr.value = result; + return result; + } + return null; + } + default: + return null; + } + } case "BinaryExpression": { const lhsValue = read(constants, value.left); const rhsValue = read(constants, value.right); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.expect.md new file mode 100644 index 0000000000..50b1a10a24 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.expect.md @@ -0,0 +1,86 @@ + +## Input + +```javascript +import { Stringify } from "shared-runtime"; + +function foo() { + let _b; + const b = true; + if (!b) { + _b = "bar"; + } else { + _b = "baz"; + } + + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +function foo() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = ( + + ); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; + +``` + +### Eval output +(kind: ok)
{"value":{"_b":"baz","b0":false,"n0":true,"n1":false,"n2":false,"n3":false,"s0":true,"s1":false,"s2":false,"u":true,"n":true}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.js new file mode 100644 index 0000000000..f952bb1c2d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.js @@ -0,0 +1,35 @@ +import { Stringify } from "shared-runtime"; + +function foo() { + let _b; + const b = true; + if (!b) { + _b = "bar"; + } else { + _b = "baz"; + } + + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; From 867edc6576956f577718540c504a10bcfe2aad77 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 29 May 2024 07:56:27 -0700 Subject: [PATCH 26/26] compiler: ValidateNoRefInRender detects writes of refs Improves ValidateNoRefAccessInRender, detecting modifications of refs during render. Fixes #29161 ghstack-source-id: 99078b3cea5b2d9019dbf77ede9c2e4cd9fbfd27 Pull Request resolved: https://github.com/facebook/react/pull/29170 --- .../Validation/ValidateNoRefAccesInRender.ts | 59 +++++++++++++++---- ...-callback-invoked-during-render-.expect.md | 2 +- ...ror.invalid-pass-ref-to-function.expect.md | 2 +- ...n-callback-invoked-during-render.expect.md | 2 +- ...d-set-and-read-ref-during-render.expect.md | 9 ++- ...ef-nested-property-during-render.expect.md | 29 +++++++++ ...-read-ref-nested-property-during-render.js | 6 ++ ...f-added-to-dep-without-type-info.expect.md | 2 +- ...rite-but-dont-read-ref-in-render.expect.md | 29 +++++++++ ...valid-write-but-dont-read-ref-in-render.js | 8 +++ ...alidate-mutate-ref-arg-in-render.expect.md | 2 +- 11 files changed, 132 insertions(+), 18 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts index ee138605be..f0c3fdf2cb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts @@ -10,6 +10,7 @@ import { HIRFunction, IdentifierId, Place, + SourceLocation, isRefValueType, isUseRefType, } from "../HIR"; @@ -117,7 +118,12 @@ function validateNoRefAccessInRenderImpl( case "MethodCall": { if (!isEffectHook(instr.value.property.identifier)) { for (const operand of eachInstructionValueOperand(instr.value)) { - validateNoRefAccess(errors, refAccessingFunctions, operand); + validateNoRefAccess( + errors, + refAccessingFunctions, + operand, + operand.loc + ); } } break; @@ -138,7 +144,12 @@ function validateNoRefAccessInRenderImpl( }); } for (const operand of eachInstructionValueOperand(instr.value)) { - validateNoRefAccess(errors, refAccessingFunctions, operand); + validateNoRefAccess( + errors, + refAccessingFunctions, + operand, + operand.loc + ); } } break; @@ -146,7 +157,30 @@ function validateNoRefAccessInRenderImpl( case "ObjectExpression": case "ArrayExpression": { for (const operand of eachInstructionValueOperand(instr.value)) { - validateNoRefAccess(errors, refAccessingFunctions, operand); + validateNoRefAccess( + errors, + refAccessingFunctions, + operand, + operand.loc + ); + } + break; + } + case "PropertyDelete": + case "PropertyStore": + case "ComputedDelete": + case "ComputedStore": { + validateNoRefAccess( + errors, + refAccessingFunctions, + instr.value.object, + instr.loc + ); + for (const operand of eachInstructionValueOperand(instr.value)) { + if (operand === instr.value.object) { + continue; + } + validateNoRefValueAccess(errors, refAccessingFunctions, operand); } break; } @@ -172,12 +206,12 @@ function validateNoRefAccessInRenderImpl( function validateNoRefValueAccess( errors: CompilerError, - unconditionalSetStateFunctions: Set, + refAccessingFunctions: Set, operand: Place ): void { if ( isRefValueType(operand.identifier) || - unconditionalSetStateFunctions.has(operand.identifier.id) + refAccessingFunctions.has(operand.identifier.id) ) { errors.push({ severity: ErrorSeverity.InvalidReact, @@ -192,20 +226,25 @@ function validateNoRefValueAccess( function validateNoRefAccess( errors: CompilerError, - unconditionalSetStateFunctions: Set, - operand: Place + refAccessingFunctions: Set, + operand: Place, + loc: SourceLocation ): void { if ( isRefValueType(operand.identifier) || isUseRefType(operand.identifier) || - unconditionalSetStateFunctions.has(operand.identifier.id) + refAccessingFunctions.has(operand.identifier.id) ) { errors.push({ severity: ErrorSeverity.InvalidReact, reason: "Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)", - loc: operand.loc, - description: `Cannot access ref value at ${printPlace(operand)}`, + loc: loc, + description: + operand.identifier.name !== null && + operand.identifier.name.kind === "named" + ? `Cannot access ref value \`${operand.identifier.name.value}\`` + : null, suggestions: null, }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md index 9560bd708f..b8ceb6cd0f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md @@ -22,7 +22,7 @@ function Component(props) { 7 | return ; 8 | }; > 9 | return {props.items.map((item) => renderItem(item))}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at mutate? $64[13:15]:TObject (9:9) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9) 10 | } 11 | ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-pass-ref-to-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-pass-ref-to-function.expect.md index 9412d21ddf..27c116e346 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-pass-ref-to-function.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-pass-ref-to-function.expect.md @@ -18,7 +18,7 @@ function Component(props) { 2 | function Component(props) { 3 | const ref = useRef(null); > 4 | const x = foo(ref); - | ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at mutate? $21[6:8]:TObject (4:4) + | ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4) 5 | return x.current; 6 | } 7 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md index 0118d7d1e1..dd8b25e505 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md @@ -21,7 +21,7 @@ function Component(props) { 6 | return ; 7 | }; > 8 | return {props.items.map((item) => renderItem(item))}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at mutate? $60[14:16]:TObject (8:8) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (8:8) 9 | } 10 | ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-during-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-during-render.expect.md index 8e35021703..5db1568427 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-during-render.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-during-render.expect.md @@ -15,10 +15,13 @@ function Component(props) { ## Error ``` + 2 | function Component(props) { 3 | const ref = useRef(null); - 4 | ref.current = props.value; -> 5 | return ref.current; - | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $24:TObject (5:5) +> 4 | ref.current = props.value; + | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4) + +InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $24:TObject (5:5) + 5 | return ref.current; 6 | } 7 | ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.expect.md new file mode 100644 index 0000000000..4d8cb2ae7c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender +function Component(props) { + const ref = useRef({ inner: null }); + ref.current.inner = props.value; + return ref.current.inner; +} + +``` + + +## Error + +``` + 2 | function Component(props) { + 3 | const ref = useRef({ inner: null }); +> 4 | ref.current.inner = props.value; + | ^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4) + +InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $30:TObject (5:5) + 5 | return ref.current.inner; + 6 | } + 7 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.js new file mode 100644 index 0000000000..6e527c46d3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.js @@ -0,0 +1,6 @@ +// @validateRefAccessDuringRender +function Component(props) { + const ref = useRef({ inner: null }); + ref.current.inner = props.value; + return ref.current.inner; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md index 44cacf48d1..e4a97deafd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md @@ -25,7 +25,7 @@ function Foo({ a }) { 3 | const ref = useRef(); 4 | // type information is lost here as we don't track types of fields > 5 | const val = { ref }; - | ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at capture $29:TObject (5:5) + | ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5) 6 | // without type info, we don't know that val.ref.current is a ref value so we 7 | // *would* end up depending on val.ref.current 8 | // however, this is an instance of accessing a ref during render and is disallowed diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.expect.md new file mode 100644 index 0000000000..7b57eeab4d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender +function useHook({ value }) { + const ref = useRef(null); + // Writing to a ref in render is against the rules: + ref.current = value; + // returning a ref is allowed, so this alone doesn't trigger an error: + return ref; +} + +``` + + +## Error + +``` + 3 | const ref = useRef(null); + 4 | // Writing to a ref in render is against the rules: +> 5 | ref.current = value; + | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5) + 6 | // returning a ref is allowed, so this alone doesn't trigger an error: + 7 | return ref; + 8 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.js new file mode 100644 index 0000000000..613b087829 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.js @@ -0,0 +1,8 @@ +// @validateRefAccessDuringRender +function useHook({ value }) { + const ref = useRef(null); + // Writing to a ref in render is against the rules: + ref.current = value; + // returning a ref is allowed, so this alone doesn't trigger an error: + return ref; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md index a1706958e7..3d47d6e247 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md @@ -23,7 +23,7 @@ export const FIXTURE_ENTRYPOINT = { 1 | // @validateRefAccessDuringRender:true 2 | function Foo(props, ref) { > 3 | console.log(ref.current); - | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at read $16:TObject (3:3) + | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (3:3) 4 | return
{props.bar}
; 5 | } 6 |