diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/01-user-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/01-user-output.txt index ba680bbb57232..1600f35107339 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/01-user-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/01-user-output.txt @@ -1,4 +1,5 @@ -function TestComponent(t0) { +import { c as _c } from "react/compiler-runtime"; +export default function TestComponent(t0) { const $ = _c(2); const { x } = t0; let t1; diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/02-default-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/02-default-output.txt index 2cbd09bba6179..1d59a120f9849 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/02-default-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/02-default-output.txt @@ -1,4 +1,5 @@ -function MyApp() { +import { c as _c } from "react/compiler-runtime"; +export default function MyApp() { const $ = _c(1); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-memo-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-memo-output.txt index ba680bbb57232..638a2bcd22841 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-memo-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-memo-output.txt @@ -1,4 +1,6 @@ -function TestComponent(t0) { +"use memo"; +import { c as _c } from "react/compiler-runtime"; +export default function TestComponent(t0) { const $ = _c(2); const { x } = t0; let t1; diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-no-memo-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-no-memo-output.txt index 2c69ddc1d65b8..ebd2d2b04678c 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-no-memo-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-no-memo-output.txt @@ -1,3 +1,4 @@ -function TestComponent({ x }) { +"use no memo"; +export default function TestComponent({ x }) { return ; } diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-flow-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-flow-output.txt new file mode 100644 index 0000000000000..c0907856babf7 --- /dev/null +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-flow-output.txt @@ -0,0 +1,14 @@ +import { c as _c } from "react/compiler-runtime"; +function useFoo(propVal) { +  const $ = _c(2); +  const t0 = (propVal.baz: number); +  let t1; +  if ($[0] !== t0) { +    t1 = 
{t0}
; +    $[0] = t0; +    $[1] = t1; +  } else { +    t1 = $[1]; +  } +  return t1; +} \ No newline at end of file diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-typescript-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-typescript-output.txt new file mode 100644 index 0000000000000..c936c5ae2ed0a --- /dev/null +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-typescript-output.txt @@ -0,0 +1,20 @@ +import { c as _c } from "react/compiler-runtime"; +function Foo() { +  const $ = _c(2); +  let t0; +  if ($[0] === Symbol.for("react.memo_cache_sentinel")) { +    t0 = foo(); +    $[0] = t0; +  } else { +    t0 = $[0]; +  } +  const x = t0 as number; +  let t1; +  if ($[1] === Symbol.for("react.memo_cache_sentinel")) { +    t1 = 
{x}
; +    $[1] = t1; +  } else { +    t1 = $[1]; +  } +  return t1; +} \ No newline at end of file diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/todo-function-scope-does-not-beat-module-scope-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/todo-function-scope-does-not-beat-module-scope-output.txt new file mode 100644 index 0000000000000..325e6972e1514 --- /dev/null +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/todo-function-scope-does-not-beat-module-scope-output.txt @@ -0,0 +1,5 @@ +"use no memo"; +function TestComponent({ x }) { + "use memo"; + return ; +} diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-memo-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-memo-output.txt index 804bacab97e05..de6dd52680077 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-memo-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-memo-output.txt @@ -1,3 +1,4 @@ +import { c as _c } from "react/compiler-runtime"; function TestComponent(t0) { "use memo"; const $ = _c(2); @@ -12,7 +13,7 @@ function TestComponent(t0) { } return t1; } -function anonymous_1(t0) { +const TestComponent2 = (t0) => { "use memo"; const $ = _c(2); const { x } = t0; @@ -25,4 +26,4 @@ function anonymous_1(t0) { t1 = $[1]; } return t1; -} +}; diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-no-memo-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-no-memo-output.txt index 5fb66309fc70c..02c1367622185 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-no-memo-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-no-memo-output.txt @@ -1,8 +1,8 @@ -function anonymous_1() { +const TestComponent = function () { "use no memo"; return ; -} -function anonymous_3({ x }) { +}; +const TestComponent2 = ({ x }) => { "use no memo"; return ; -} +}; diff --git a/compiler/apps/playground/__tests__/e2e/page.spec.ts b/compiler/apps/playground/__tests__/e2e/page.spec.ts index 846e6227bd1a1..05fe96d4b9048 100644 --- a/compiler/apps/playground/__tests__/e2e/page.spec.ts +++ b/compiler/apps/playground/__tests__/e2e/page.spec.ts @@ -9,11 +9,11 @@ import {expect, test} from '@playwright/test'; import {encodeStore, type Store} from '../../lib/stores'; import {format} from 'prettier'; -function print(data: Array): Promise { +function formatPrint(data: Array): Promise { return format(data.join(''), {parser: 'babel'}); } -const DIRECTIVE_TEST_CASES = [ +const TEST_CASE_INPUTS = [ { name: 'module-scope-use-memo', input: ` @@ -55,7 +55,7 @@ const TestComponent2 = ({ x }) => { };`, }, { - name: 'function-scope-beats-module-scope', + name: 'todo-function-scope-does-not-beat-module-scope', input: ` 'use no memo'; function TestComponent({ x }) { @@ -63,6 +63,26 @@ function TestComponent({ x }) { return ; }`, }, + { + name: 'parse-typescript', + input: ` +function Foo() { + const x = foo() as number; + return
{x}
; +} +`, + noFormat: true, + }, + { + name: 'parse-flow', + input: ` +// @flow +function useFoo(propVal: {+baz: number}) { + return
{(propVal.baz as number)}
; +} + `, + noFormat: true, + }, ]; test('editor should open successfully', async ({page}) => { @@ -90,7 +110,7 @@ test('editor should compile from hash successfully', async ({page}) => { }); const text = (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? []; - const output = await print(text); + const output = await formatPrint(text); expect(output).not.toEqual(''); expect(output).toMatchSnapshot('01-user-output.txt'); @@ -115,14 +135,14 @@ test('reset button works', async ({page}) => { }); const text = (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? []; - const output = await print(text); + const output = await formatPrint(text); expect(output).not.toEqual(''); expect(output).toMatchSnapshot('02-default-output.txt'); }); -DIRECTIVE_TEST_CASES.forEach((t, idx) => - test(`directives work: ${t.name}`, async ({page}) => { +TEST_CASE_INPUTS.forEach((t, idx) => + test(`playground compiles: ${t.name}`, async ({page}) => { const store: Store = { source: t.input, }; @@ -135,7 +155,12 @@ DIRECTIVE_TEST_CASES.forEach((t, idx) => const text = (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? []; - const output = await print(text); + let output: string; + if (t.noFormat) { + output = text.join(''); + } else { + output = await formatPrint(text); + } expect(output).not.toEqual(''); expect(output).toMatchSnapshot(`${t.name}-output.txt`); diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index 82a40272bd312..785b9fd075d12 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -5,23 +5,22 @@ * LICENSE file in the root directory of this source tree. */ -import {parse as babelParse} from '@babel/parser'; +import {parse as babelParse, ParseResult} from '@babel/parser'; import * as HermesParser from 'hermes-parser'; -import traverse, {NodePath} from '@babel/traverse'; import * as t from '@babel/types'; -import { +import BabelPluginReactCompiler, { CompilerError, CompilerErrorDetail, Effect, ErrorSeverity, parseConfigPragmaForTests, ValueKind, - runPlayground, type Hook, - findDirectiveDisablingMemoization, - findDirectiveEnablingMemoization, + PluginOptions, + CompilerPipelineValue, + parsePluginOptions, } from 'babel-plugin-react-compiler/src'; -import {type ReactFunctionType} from 'babel-plugin-react-compiler/src/HIR/Environment'; +import {type EnvironmentConfig} from 'babel-plugin-react-compiler/src/HIR/Environment'; import clsx from 'clsx'; import invariant from 'invariant'; import {useSnackbar} from 'notistack'; @@ -39,32 +38,18 @@ import {useStore, useStoreDispatch} from '../StoreContext'; import Input from './Input'; import { CompilerOutput, + CompilerTransformOutput, default as Output, PrintedCompilerPipelineValue, } from './Output'; import {printFunctionWithOutlined} from 'babel-plugin-react-compiler/src/HIR/PrintHIR'; import {printReactiveFunctionWithOutlined} from 'babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction'; +import {transformFromAstSync} from '@babel/core'; -type FunctionLike = - | NodePath - | NodePath - | NodePath; -enum MemoizeDirectiveState { - Enabled = 'Enabled', - Disabled = 'Disabled', - Undefined = 'Undefined', -} - -const MEMOIZE_ENABLED_OR_UNDEFINED_STATES = new Set([ - MemoizeDirectiveState.Enabled, - MemoizeDirectiveState.Undefined, -]); - -const MEMOIZE_ENABLED_OR_DISABLED_STATES = new Set([ - MemoizeDirectiveState.Enabled, - MemoizeDirectiveState.Disabled, -]); -function parseInput(input: string, language: 'flow' | 'typescript'): any { +function parseInput( + input: string, + language: 'flow' | 'typescript', +): ParseResult { // Extract the first line to quickly check for custom test directives if (language === 'flow') { return HermesParser.parse(input, { @@ -77,95 +62,45 @@ function parseInput(input: string, language: 'flow' | 'typescript'): any { return babelParse(input, { plugins: ['typescript', 'jsx'], sourceType: 'module', - }); + }) as ParseResult; } } -function parseFunctions( +function invokeCompiler( source: string, language: 'flow' | 'typescript', -): Array<{ - compilationEnabled: boolean; - fn: FunctionLike; -}> { - const items: Array<{ - compilationEnabled: boolean; - fn: FunctionLike; - }> = []; - try { - const ast = parseInput(source, language); - traverse(ast, { - FunctionDeclaration(nodePath) { - items.push({ - compilationEnabled: shouldCompile(nodePath), - fn: nodePath, - }); - nodePath.skip(); - }, - ArrowFunctionExpression(nodePath) { - items.push({ - compilationEnabled: shouldCompile(nodePath), - fn: nodePath, - }); - nodePath.skip(); - }, - FunctionExpression(nodePath) { - items.push({ - compilationEnabled: shouldCompile(nodePath), - fn: nodePath, - }); - nodePath.skip(); - }, - }); - } catch (e) { - console.error(e); - CompilerError.throwInvalidJS({ - reason: String(e), - description: null, - loc: null, - suggestions: null, - }); - } - - return items; -} - -function shouldCompile(fn: FunctionLike): boolean { - const {body} = fn.node; - if (t.isBlockStatement(body)) { - const selfCheck = checkExplicitMemoizeDirectives(body.directives); - if (selfCheck === MemoizeDirectiveState.Enabled) return true; - if (selfCheck === MemoizeDirectiveState.Disabled) return false; - - const parentWithDirective = fn.findParent(parentPath => { - if (parentPath.isBlockStatement() || parentPath.isProgram()) { - const directiveCheck = checkExplicitMemoizeDirectives( - parentPath.node.directives, - ); - return MEMOIZE_ENABLED_OR_DISABLED_STATES.has(directiveCheck); - } - return false; - }); - - if (!parentWithDirective) return true; - const parentDirectiveCheck = checkExplicitMemoizeDirectives( - (parentWithDirective.node as t.Program | t.BlockStatement).directives, - ); - return MEMOIZE_ENABLED_OR_UNDEFINED_STATES.has(parentDirectiveCheck); - } - return false; -} - -function checkExplicitMemoizeDirectives( - directives: Array, -): MemoizeDirectiveState { - if (findDirectiveEnablingMemoization(directives).length) { - return MemoizeDirectiveState.Enabled; - } - if (findDirectiveDisablingMemoization(directives).length) { - return MemoizeDirectiveState.Disabled; + environment: EnvironmentConfig, + logIR: (pipelineValue: CompilerPipelineValue) => void, +): CompilerTransformOutput { + const opts: PluginOptions = parsePluginOptions({ + logger: { + debugLogIRs: logIR, + logEvent: () => {}, + }, + environment, + compilationMode: 'all', + panicThreshold: 'all_errors', + }); + const ast = parseInput(source, language); + let result = transformFromAstSync(ast, source, { + filename: '_playgroundFile.js', + highlightCode: false, + retainLines: true, + plugins: [[BabelPluginReactCompiler, opts]], + ast: true, + sourceType: 'module', + configFile: false, + sourceMaps: true, + babelrc: false, + }); + if (result?.ast == null || result?.code == null || result?.map == null) { + throw new Error('Expected successful compilation'); } - return MemoizeDirectiveState.Undefined; + return { + code: result.code, + sourceMaps: result.map, + language, + }; } const COMMON_HOOKS: Array<[string, Hook]> = [ @@ -216,37 +151,6 @@ const COMMON_HOOKS: Array<[string, Hook]> = [ ], ]; -function isHookName(s: string): boolean { - return /^use[A-Z0-9]/.test(s); -} - -function getReactFunctionType(id: t.Identifier | null): ReactFunctionType { - if (id != null) { - if (isHookName(id.name)) { - return 'Hook'; - } - - const isPascalCaseNameSpace = /^[A-Z].*/; - if (isPascalCaseNameSpace.test(id.name)) { - return 'Component'; - } - } - return 'Other'; -} - -function getFunctionIdentifier( - fn: - | NodePath - | NodePath - | NodePath, -): t.Identifier | null { - if (fn.isArrowFunctionExpression()) { - return null; - } - const id = fn.get('id'); - return Array.isArray(id) === false && id.isIdentifier() ? id.node : null; -} - function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { const results = new Map>(); const error = new CompilerError(); @@ -264,71 +168,25 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { } else { language = 'typescript'; } - let count = 0; - const withIdentifier = (id: t.Identifier | null): t.Identifier => { - if (id != null && id.name != null) { - return id; - } else { - return t.identifier(`anonymous_${count++}`); - } - }; + let transformOutput; try { // Extract the first line to quickly check for custom test directives const pragma = source.substring(0, source.indexOf('\n')); const config = parseConfigPragmaForTests(pragma); - const parsedFunctions = parseFunctions(source, language); - for (const func of parsedFunctions) { - const id = withIdentifier(getFunctionIdentifier(func.fn)); - const fnName = id.name; - if (!func.compilationEnabled) { - upsert({ - kind: 'ast', - fnName, - name: 'CodeGen', - value: { - type: 'FunctionDeclaration', - id: - func.fn.isArrowFunctionExpression() || - func.fn.isFunctionExpression() - ? withIdentifier(null) - : func.fn.node.id, - async: func.fn.node.async, - generator: !!func.fn.node.generator, - body: func.fn.node.body as t.BlockStatement, - params: func.fn.node.params, - }, - }); - continue; - } - for (const result of runPlayground( - func.fn, - { - ...config, - customHooks: new Map([...COMMON_HOOKS]), - }, - getReactFunctionType(id), - )) { + + transformOutput = invokeCompiler( + source, + language, + {...config, customHooks: new Map([...COMMON_HOOKS])}, + result => { switch (result.kind) { case 'ast': { - upsert({ - kind: 'ast', - fnName, - name: result.name, - value: { - type: 'FunctionDeclaration', - id: withIdentifier(result.value.id), - async: result.value.async, - generator: result.value.generator, - body: result.value.body, - params: result.value.params, - }, - }); break; } case 'hir': { upsert({ kind: 'hir', - fnName, + fnName: result.value.id, name: result.name, value: printFunctionWithOutlined(result.value), }); @@ -337,7 +195,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { case 'reactive': { upsert({ kind: 'reactive', - fnName, + fnName: result.value.id, name: result.name, value: printReactiveFunctionWithOutlined(result.value), }); @@ -346,7 +204,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { case 'debug': { upsert({ kind: 'debug', - fnName, + fnName: null, name: result.name, value: result.value, }); @@ -357,8 +215,8 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { throw new Error(`Unhandled result ${result}`); } } - } - } + }, + ); } catch (err) { /** * error might be an invariant violation or other runtime error @@ -385,7 +243,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { if (error.hasErrors()) { return [{kind: 'err', results, error: error}, language]; } - return [{kind: 'ok', results}, language]; + return [{kind: 'ok', results, transformOutput}, language]; } export default function Editor(): JSX.Element { @@ -405,7 +263,7 @@ export default function Editor(): JSX.Element { } catch (e) { invariant(e instanceof Error, 'Only Error may be caught.'); enqueueSnackbar(e.message, { - variant: 'message', + variant: 'warning', ...createMessage( 'Bad URL - fell back to the default Playground.', MessageLevel.Info, diff --git a/compiler/apps/playground/components/Editor/Output.tsx b/compiler/apps/playground/components/Editor/Output.tsx index 97a63400d2b42..d4127c63cff05 100644 --- a/compiler/apps/playground/components/Editor/Output.tsx +++ b/compiler/apps/playground/components/Editor/Output.tsx @@ -5,8 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import generate from '@babel/generator'; -import * as t from '@babel/types'; import { CodeIcon, DocumentAddIcon, @@ -21,17 +19,12 @@ import {memo, ReactNode, useEffect, useState} from 'react'; import {type Store} from '../../lib/stores'; import TabbedWindow from '../TabbedWindow'; import {monacoOptions} from './monacoOptions'; +import {BabelFileResult} from '@babel/core'; const MemoizedOutput = memo(Output); export default MemoizedOutput; export type PrintedCompilerPipelineValue = - | { - kind: 'ast'; - name: string; - fnName: string | null; - value: t.FunctionDeclaration; - } | { kind: 'hir'; name: string; @@ -41,8 +34,17 @@ export type PrintedCompilerPipelineValue = | {kind: 'reactive'; name: string; fnName: string | null; value: string} | {kind: 'debug'; name: string; fnName: string | null; value: string}; +export type CompilerTransformOutput = { + code: string; + sourceMaps: BabelFileResult['map']; + language: 'flow' | 'typescript'; +}; export type CompilerOutput = - | {kind: 'ok'; results: Map>} + | { + kind: 'ok'; + transformOutput: CompilerTransformOutput; + results: Map>; + } | { kind: 'err'; results: Map>; @@ -61,7 +63,6 @@ async function tabify( const tabs = new Map(); const reorderedTabs = new Map(); const concattedResults = new Map(); - let topLevelFnDecls: Array = []; // Concat all top level function declaration results into a single tab for each pass for (const [passName, results] of compilerOutput.results) { for (const result of results) { @@ -87,9 +88,6 @@ async function tabify( } break; } - case 'ast': - topLevelFnDecls.push(result.value); - break; case 'debug': { concattedResults.set(passName, result.value); break; @@ -114,13 +112,17 @@ async function tabify( lastPassOutput = text; } // Ensure that JS and the JS source map come first - if (topLevelFnDecls.length > 0) { - /** - * Make a synthetic Program so we can have a single AST with all the top level - * FunctionDeclarations - */ - const ast = t.program(topLevelFnDecls); - const {code, sourceMapUrl} = await codegen(ast, source); + if (compilerOutput.kind === 'ok') { + const {transformOutput} = compilerOutput; + const sourceMapUrl = getSourceMapUrl( + transformOutput.code, + JSON.stringify(transformOutput.sourceMaps), + ); + const code = await prettier.format(transformOutput.code, { + semi: true, + parser: transformOutput.language === 'flow' ? 'babel-flow' : 'babel-ts', + plugins: [parserBabel, prettierPluginEstree], + }); reorderedTabs.set( 'JS', { - const generated = generate( - ast, - {sourceMaps: true, sourceFileName: 'input.js'}, - source, - ); - const sourceMapUrl = getSourceMapUrl( - generated.code, - JSON.stringify(generated.map), - ); - const codegenOutput = await prettier.format(generated.code, { - semi: true, - parser: 'babel', - plugins: [parserBabel, prettierPluginEstree], - }); - return {code: codegenOutput, sourceMapUrl}; -} - function utf16ToUTF8(s: string): string { return unescape(encodeURIComponent(s)); } diff --git a/compiler/packages/babel-plugin-react-compiler/package.json b/compiler/packages/babel-plugin-react-compiler/package.json index 7d55e7b27fe9b..158b800dba900 100644 --- a/compiler/packages/babel-plugin-react-compiler/package.json +++ b/compiler/packages/babel-plugin-react-compiler/package.json @@ -42,9 +42,7 @@ "babel-jest": "^29.0.3", "babel-plugin-fbt": "^1.0.0", "babel-plugin-fbt-runtime": "^1.0.0", - "chalk": "4", "eslint": "^8.57.1", - "glob": "^7.1.6", "invariant": "^2.2.4", "jest": "^29.0.3", "jest-environment-jsdom": "^29.0.3", diff --git a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts index 401cbd4bdf50f..c648c66043980 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts @@ -39,7 +39,10 @@ export default function BabelPluginReactCompiler( ) { opts = injectReanimatedFlag(opts); } - if (isDev) { + if ( + opts.environment.enableResetCacheOnSourceFileChanges !== false && + isDev + ) { opts = { ...opts, environment: { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts index 72ed9e7c866ce..fb951d25c5398 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts @@ -15,6 +15,7 @@ import { } from '../HIR/Environment'; import {hasOwnProperty} from '../Utils/utils'; import {fromZodError} from 'zod-validation-error'; +import {CompilerPipelineValue} from './Pipeline'; const PanicThresholdOptionsSchema = z.enum([ /* @@ -209,6 +210,7 @@ export type LoggerEvent = export type Logger = { logEvent: (filename: string | null, event: LoggerEvent) => void; + debugLogIRs?: (value: CompilerPipelineValue) => void; }; export const defaultOptions: PluginOptions = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 90921454c864f..c48cba32b2642 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -79,13 +79,6 @@ import { rewriteInstructionKindsBasedOnReassignment, } from '../SSA'; import {inferTypes} from '../TypeInference'; -import { - logCodegenFunction, - logDebug, - logHIRFunction, - logReactiveFunction, -} from '../Utils/logger'; -import {assertExhaustive} from '../Utils/utils'; import { validateContextVariableLValues, validateHooksUsage, @@ -104,6 +97,7 @@ import {validateNoSetStateInPassiveEffects} from '../Validation/ValidateNoSetSta import {validateNoJSXInTryStatement} from '../Validation/ValidateNoJSXInTryStatement'; import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHIR'; import {outlineJSX} from '../Optimization/OutlineJsx'; +import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -111,7 +105,7 @@ export type CompilerPipelineValue = | {kind: 'reactive'; name: string; value: ReactiveFunction} | {kind: 'debug'; name: string; value: string}; -export function* run( +function run( func: NodePath< t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression >, @@ -121,7 +115,7 @@ export function* run( logger: Logger | null, filename: string | null, code: string | null, -): Generator { +): CodegenFunction { const contextIdentifiers = findContextIdentifiers(func); const env = new Environment( func.scope, @@ -133,30 +127,32 @@ export function* run( code, useMemoCacheIdentifier, ); - yield log({ + env.logger?.debugLogIRs?.({ kind: 'debug', name: 'EnvironmentConfig', value: prettyFormat(env.config), }); - const ast = yield* runWithEnvironment(func, env); - return ast; + return runWithEnvironment(func, env); } /* * Note: this is split from run() to make `config` out of scope, so that all * access to feature flags has to be through the Environment for consistency. */ -function* runWithEnvironment( +function runWithEnvironment( func: NodePath< t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression >, env: Environment, -): Generator { +): CodegenFunction { + const log = (value: CompilerPipelineValue): void => { + env.logger?.debugLogIRs?.(value); + }; const hir = lower(func, env).unwrap(); - yield log({kind: 'hir', name: 'HIR', value: hir}); + log({kind: 'hir', name: 'HIR', value: hir}); pruneMaybeThrows(hir); - yield log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); + log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); validateContextVariableLValues(hir); validateUseMemo(hir); @@ -167,35 +163,35 @@ function* runWithEnvironment( !env.config.enableChangeDetectionForDebugging ) { dropManualMemoization(hir); - yield log({kind: 'hir', name: 'DropManualMemoization', value: hir}); + log({kind: 'hir', name: 'DropManualMemoization', value: hir}); } inlineImmediatelyInvokedFunctionExpressions(hir); - yield log({ + log({ kind: 'hir', name: 'InlineImmediatelyInvokedFunctionExpressions', value: hir, }); mergeConsecutiveBlocks(hir); - yield log({kind: 'hir', name: 'MergeConsecutiveBlocks', value: hir}); + log({kind: 'hir', name: 'MergeConsecutiveBlocks', value: hir}); assertConsistentIdentifiers(hir); assertTerminalSuccessorsExist(hir); enterSSA(hir); - yield log({kind: 'hir', name: 'SSA', value: hir}); + log({kind: 'hir', name: 'SSA', value: hir}); eliminateRedundantPhi(hir); - yield log({kind: 'hir', name: 'EliminateRedundantPhi', value: hir}); + log({kind: 'hir', name: 'EliminateRedundantPhi', value: hir}); assertConsistentIdentifiers(hir); constantPropagation(hir); - yield log({kind: 'hir', name: 'ConstantPropagation', value: hir}); + log({kind: 'hir', name: 'ConstantPropagation', value: hir}); inferTypes(hir); - yield log({kind: 'hir', name: 'InferTypes', value: hir}); + log({kind: 'hir', name: 'InferTypes', value: hir}); if (env.config.validateHooksUsage) { validateHooksUsage(hir); @@ -209,28 +205,31 @@ function* runWithEnvironment( lowerContextAccess(hir, env.config.lowerContextAccess); } + optimizePropsMethodCalls(hir); + log({kind: 'hir', name: 'OptimizePropsMethodCalls', value: hir}); + analyseFunctions(hir); - yield log({kind: 'hir', name: 'AnalyseFunctions', value: hir}); + log({kind: 'hir', name: 'AnalyseFunctions', value: hir}); inferReferenceEffects(hir); - yield log({kind: 'hir', name: 'InferReferenceEffects', value: hir}); + log({kind: 'hir', name: 'InferReferenceEffects', value: hir}); validateLocalsNotReassignedAfterRender(hir); // Note: Has to come after infer reference effects because "dead" code may still affect inference deadCodeElimination(hir); - yield log({kind: 'hir', name: 'DeadCodeElimination', value: hir}); + log({kind: 'hir', name: 'DeadCodeElimination', value: hir}); if (env.config.enableInstructionReordering) { instructionReordering(hir); - yield log({kind: 'hir', name: 'InstructionReordering', value: hir}); + log({kind: 'hir', name: 'InstructionReordering', value: hir}); } pruneMaybeThrows(hir); - yield log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); + log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); inferMutableRanges(hir); - yield log({kind: 'hir', name: 'InferMutableRanges', value: hir}); + log({kind: 'hir', name: 'InferMutableRanges', value: hir}); if (env.config.assertValidMutableRanges) { assertValidMutableRanges(hir); @@ -253,27 +252,27 @@ function* runWithEnvironment( } inferReactivePlaces(hir); - yield log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); + log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); rewriteInstructionKindsBasedOnReassignment(hir); - yield log({ + log({ kind: 'hir', name: 'RewriteInstructionKindsBasedOnReassignment', value: hir, }); propagatePhiTypes(hir); - yield log({ + log({ kind: 'hir', name: 'PropagatePhiTypes', value: hir, }); inferReactiveScopeVariables(hir); - yield log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir}); + log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir}); const fbtOperands = memoizeFbtAndMacroOperandsInSameScope(hir); - yield log({ + log({ kind: 'hir', name: 'MemoizeFbtAndMacroOperandsInSameScope', value: hir, @@ -285,39 +284,39 @@ function* runWithEnvironment( if (env.config.enableFunctionOutlining) { outlineFunctions(hir, fbtOperands); - yield log({kind: 'hir', name: 'OutlineFunctions', value: hir}); + log({kind: 'hir', name: 'OutlineFunctions', value: hir}); } alignMethodCallScopes(hir); - yield log({ + log({ kind: 'hir', name: 'AlignMethodCallScopes', value: hir, }); alignObjectMethodScopes(hir); - yield log({ + log({ kind: 'hir', name: 'AlignObjectMethodScopes', value: hir, }); pruneUnusedLabelsHIR(hir); - yield log({ + log({ kind: 'hir', name: 'PruneUnusedLabelsHIR', value: hir, }); alignReactiveScopesToBlockScopesHIR(hir); - yield log({ + log({ kind: 'hir', name: 'AlignReactiveScopesToBlockScopesHIR', value: hir, }); mergeOverlappingReactiveScopesHIR(hir); - yield log({ + log({ kind: 'hir', name: 'MergeOverlappingReactiveScopesHIR', value: hir, @@ -325,7 +324,7 @@ function* runWithEnvironment( assertValidBlockNesting(hir); buildReactiveScopeTerminalsHIR(hir); - yield log({ + log({ kind: 'hir', name: 'BuildReactiveScopeTerminalsHIR', value: hir, @@ -334,14 +333,14 @@ function* runWithEnvironment( assertValidBlockNesting(hir); flattenReactiveLoopsHIR(hir); - yield log({ + log({ kind: 'hir', name: 'FlattenReactiveLoopsHIR', value: hir, }); flattenScopesWithHooksOrUseHIR(hir); - yield log({ + log({ kind: 'hir', name: 'FlattenScopesWithHooksOrUseHIR', value: hir, @@ -349,7 +348,7 @@ function* runWithEnvironment( assertTerminalSuccessorsExist(hir); assertTerminalPredsExist(hir); propagateScopeDependenciesHIR(hir); - yield log({ + log({ kind: 'hir', name: 'PropagateScopeDependenciesHIR', value: hir, @@ -361,7 +360,7 @@ function* runWithEnvironment( if (env.config.inlineJsxTransform) { inlineJsxTransform(hir, env.config.inlineJsxTransform); - yield log({ + log({ kind: 'hir', name: 'inlineJsxTransform', value: hir, @@ -369,7 +368,7 @@ function* runWithEnvironment( } const reactiveFunction = buildReactiveFunction(hir); - yield log({ + log({ kind: 'reactive', name: 'BuildReactiveFunction', value: reactiveFunction, @@ -378,7 +377,7 @@ function* runWithEnvironment( assertWellFormedBreakTargets(reactiveFunction); pruneUnusedLabels(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneUnusedLabels', value: reactiveFunction, @@ -386,35 +385,35 @@ function* runWithEnvironment( assertScopeInstructionsWithinScopes(reactiveFunction); pruneNonEscapingScopes(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneNonEscapingScopes', value: reactiveFunction, }); pruneNonReactiveDependencies(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneNonReactiveDependencies', value: reactiveFunction, }); pruneUnusedScopes(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneUnusedScopes', value: reactiveFunction, }); mergeReactiveScopesThatInvalidateTogether(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'MergeReactiveScopesThatInvalidateTogether', value: reactiveFunction, }); pruneAlwaysInvalidatingScopes(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneAlwaysInvalidatingScopes', value: reactiveFunction, @@ -422,7 +421,7 @@ function* runWithEnvironment( if (env.config.enableChangeDetectionForDebugging != null) { pruneInitializationDependencies(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneInitializationDependencies', value: reactiveFunction, @@ -430,49 +429,49 @@ function* runWithEnvironment( } propagateEarlyReturns(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PropagateEarlyReturns', value: reactiveFunction, }); pruneUnusedLValues(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneUnusedLValues', value: reactiveFunction, }); promoteUsedTemporaries(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PromoteUsedTemporaries', value: reactiveFunction, }); extractScopeDeclarationsFromDestructuring(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'ExtractScopeDeclarationsFromDestructuring', value: reactiveFunction, }); stabilizeBlockIds(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'StabilizeBlockIds', value: reactiveFunction, }); const uniqueIdentifiers = renameVariables(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'RenameVariables', value: reactiveFunction, }); pruneHoistedContexts(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneHoistedContexts', value: reactiveFunction, @@ -493,9 +492,9 @@ function* runWithEnvironment( uniqueIdentifiers, fbtOperands, }).unwrap(); - yield log({kind: 'ast', name: 'Codegen', value: ast}); + log({kind: 'ast', name: 'Codegen', value: ast}); for (const outlined of ast.outlined) { - yield log({kind: 'ast', name: 'Codegen (outlined)', value: outlined.fn}); + log({kind: 'ast', name: 'Codegen (outlined)', value: outlined.fn}); } /** @@ -521,7 +520,7 @@ export function compileFn( filename: string | null, code: string | null, ): CodegenFunction { - let generator = run( + return run( func, config, fnType, @@ -530,46 +529,4 @@ export function compileFn( filename, code, ); - while (true) { - const next = generator.next(); - if (next.done) { - return next.value; - } - } -} - -export function log(value: CompilerPipelineValue): CompilerPipelineValue { - switch (value.kind) { - case 'ast': { - logCodegenFunction(value.name, value.value); - break; - } - case 'hir': { - logHIRFunction(value.name, value.value); - break; - } - case 'reactive': { - logReactiveFunction(value.name, value.value); - break; - } - case 'debug': { - logDebug(value.name, value.value); - break; - } - default: { - assertExhaustive(value, 'Unexpected compilation kind'); - } - } - return value; -} - -export function* runPlayground( - func: NodePath< - t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression - >, - config: EnvironmentConfig, - fnType: ReactFunctionType, -): Generator { - const ast = yield* run(func, config, fnType, '_c', null, null, null); - return ast; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index fa581d8ed8d81..e2932296ca739 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -168,11 +168,19 @@ const EnvironmentConfigSchema = z.object({ customMacros: z.nullable(z.array(MacroSchema)).default(null), /** - * Enable a check that resets the memoization cache when the source code of the file changes. - * This is intended to support hot module reloading (HMR), where the same runtime component - * instance will be reused across different versions of the component source. + * Enable a check that resets the memoization cache when the source code of + * the file changes. This is intended to support hot module reloading (HMR), + * where the same runtime component instance will be reused across different + * versions of the component source. + * + * When set to + * - true: code for HMR support is always generated, regardless of NODE_ENV + * or `globalThis.__DEV__` + * - false: code for HMR support is not generated + * - null: (default) code for HMR support is conditionally generated dependent + * on `NODE_ENV` and `globalThis.__DEV__` at the time of compilation. */ - enableResetCacheOnSourceFileChanges: z.boolean().default(false), + enableResetCacheOnSourceFileChanges: z.nullable(z.boolean()).default(null), /** * Enable using information from existing useMemo/useCallback to understand when a value is done @@ -241,6 +249,8 @@ const EnvironmentConfigSchema = z.object({ */ enableOptionalDependencies: z.boolean().default(true), + enableFire: z.boolean().default(false), + /** * Enables inference and auto-insertion of effect dependencies. Takes in an array of * configurable module and import pairs to allow for user-land experimentation. For example, @@ -708,7 +718,10 @@ export function parseConfigPragmaForTests(pragma: string): EnvironmentConfig { continue; } - if (typeof defaultConfig[key as keyof EnvironmentConfig] !== 'boolean') { + if ( + key !== 'enableResetCacheOnSourceFileChanges' && + typeof defaultConfig[key as keyof EnvironmentConfig] !== 'boolean' + ) { // skip parsing non-boolean properties continue; } @@ -718,9 +731,15 @@ export function parseConfigPragmaForTests(pragma: string): EnvironmentConfig { maybeConfig[key] = false; } } - const config = EnvironmentConfigSchema.safeParse(maybeConfig); if (config.success) { + /** + * Unless explicitly enabled, do not insert HMR handling code + * in test fixtures or playground to reduce visual noise. + */ + if (config.data.enableResetCacheOnSourceFileChanges == null) { + config.data.enableResetCacheOnSourceFileChanges = false; + } return config.data; } CompilerError.invariant(false, { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 2525b87bd86bc..ac5d44bab0875 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -9,6 +9,7 @@ import {Effect, ValueKind, ValueReason} from './HIR'; import { BUILTIN_SHAPES, BuiltInArrayId, + BuiltInFireId, BuiltInMixedReadonlyId, BuiltInUseActionStateId, BuiltInUseContextHookId, @@ -87,6 +88,21 @@ const UNTYPED_GLOBALS: Set = new Set([ ]); const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ + [ + 'Object', + addObject(DEFAULT_SHAPES, 'Object', [ + [ + 'keys', + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [Effect.Read], + restParam: null, + returnType: {kind: 'Object', shapeId: BuiltInArrayId}, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, + }), + ], + ]), + ], [ 'Array', addObject(DEFAULT_SHAPES, 'Array', [ @@ -468,6 +484,21 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ BuiltInUseOperatorId, ), ], + [ + 'fire', + addFunction( + DEFAULT_SHAPES, + [], + { + positionalParams: [], + restParam: null, + returnType: {kind: 'Primitive'}, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Frozen, + }, + BuiltInFireId, + ), + ], ]; TYPED_GLOBALS.push( 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 954fb6f40053a..a6a9cbcd48ec5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -840,6 +840,11 @@ export type LoadLocal = { place: Place; loc: SourceLocation; }; +export type LoadContext = { + kind: 'LoadContext'; + place: Place; + loc: SourceLocation; +}; /* * The value of a given instruction. Note that values are not recursive: complex @@ -852,11 +857,7 @@ export type LoadLocal = { export type InstructionValue = | LoadLocal - | { - kind: 'LoadContext'; - place: Place; - loc: SourceLocation; - } + | LoadContext | { kind: 'DeclareLocal'; lvalue: LValue; @@ -1644,6 +1645,10 @@ export function isArrayType(id: Identifier): boolean { return id.type.kind === 'Object' && id.type.shapeId === 'BuiltInArray'; } +export function isPropsType(id: Identifier): boolean { + return id.type.kind === 'Object' && id.type.shapeId === 'BuiltInProps'; +} + export function isRefValueType(id: Identifier): boolean { return id.type.kind === 'Object' && id.type.shapeId === 'BuiltInRefValue'; } 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 14f809f2c4082..4482d17890196 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -213,6 +213,7 @@ export const BuiltInDispatchId = 'BuiltInDispatch'; export const BuiltInUseContextHookId = 'BuiltInUseContextHook'; export const BuiltInUseTransitionId = 'BuiltInUseTransition'; export const BuiltInStartTransitionId = 'BuiltInStartTransition'; +export const BuiltInFireId = 'BuiltInFire'; // ShapeRegistry with default definitions for built-ins. export const BUILTIN_SHAPES: ShapeRegistry = new Map(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 8aed17f8ee847..08856e9143139 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -17,6 +17,11 @@ import { areEqualPaths, IdentifierId, Terminal, + InstructionValue, + LoadContext, + TInstruction, + FunctionExpression, + ObjectMethod, } from './HIR'; import { collectHoistablePropertyLoads, @@ -223,11 +228,25 @@ export function collectTemporariesSidemap( fn, usedOutsideDeclaringScope, temporaries, - false, + null, ); return temporaries; } +function isLoadContextMutable( + instrValue: InstructionValue, + id: InstructionId, +): instrValue is LoadContext { + if (instrValue.kind === 'LoadContext') { + CompilerError.invariant(instrValue.place.identifier.scope != null, { + reason: + '[PropagateScopeDependencies] Expected all context variables to be assigned a scope', + loc: instrValue.loc, + }); + return id >= instrValue.place.identifier.scope.range.end; + } + return false; +} /** * Recursive collect a sidemap of all `LoadLocal` and `PropertyLoads` with a * function and all nested functions. @@ -239,17 +258,21 @@ function collectTemporariesSidemapImpl( fn: HIRFunction, usedOutsideDeclaringScope: ReadonlySet, temporaries: Map, - isInnerFn: boolean, + innerFnContext: {instrId: InstructionId} | null, ): void { for (const [_, block] of fn.body.blocks) { - for (const instr of block.instructions) { - const {value, lvalue} = instr; + for (const {value, lvalue, id: origInstrId} of block.instructions) { + const instrId = + innerFnContext != null ? innerFnContext.instrId : origInstrId; const usedOutside = usedOutsideDeclaringScope.has( lvalue.identifier.declarationId, ); if (value.kind === 'PropertyLoad' && !usedOutside) { - if (!isInnerFn || temporaries.has(value.object.identifier.id)) { + if ( + innerFnContext == null || + temporaries.has(value.object.identifier.id) + ) { /** * All dependencies of a inner / nested function must have a base * identifier from the outermost component / hook. This is because the @@ -265,13 +288,13 @@ function collectTemporariesSidemapImpl( temporaries.set(lvalue.identifier.id, property); } } else if ( - value.kind === 'LoadLocal' && + (value.kind === 'LoadLocal' || isLoadContextMutable(value, instrId)) && lvalue.identifier.name == null && value.place.identifier.name !== null && !usedOutside ) { if ( - !isInnerFn || + innerFnContext == null || fn.context.some( context => context.identifier.id === value.place.identifier.id, ) @@ -289,7 +312,7 @@ function collectTemporariesSidemapImpl( value.loweredFunc.func, usedOutsideDeclaringScope, temporaries, - true, + innerFnContext ?? {instrId}, ); } } @@ -358,19 +381,22 @@ class Context { #temporaries: ReadonlyMap; #temporariesUsedOutsideScope: ReadonlySet; + #processedInstrsInOptional: ReadonlySet; /** * Tracks the traversal state. See Context.declare for explanation of why this * is needed. */ - inInnerFn: boolean = false; + #innerFnContext: {outerInstrId: InstructionId} | null = null; constructor( temporariesUsedOutsideScope: ReadonlySet, temporaries: ReadonlyMap, + processedInstrsInOptional: ReadonlySet, ) { this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope; this.#temporaries = temporaries; + this.#processedInstrsInOptional = processedInstrsInOptional; } enterScope(scope: ReactiveScope): void { @@ -431,7 +457,7 @@ class Context { * by root identifier mutable ranges). */ declare(identifier: Identifier, decl: Decl): void { - if (this.inInnerFn) return; + if (this.#innerFnContext != null) return; if (!this.#declarations.has(identifier.declarationId)) { this.#declarations.set(identifier.declarationId, decl); } @@ -574,22 +600,52 @@ class Context { currentScope.reassignments.add(place.identifier); } } + enterInnerFn( + innerFn: TInstruction | TInstruction, + cb: () => T, + ): T { + const prevContext = this.#innerFnContext; + this.#innerFnContext = this.#innerFnContext ?? {outerInstrId: innerFn.id}; + const result = cb(); + this.#innerFnContext = prevContext; + return result; + } + + /** + * Skip dependencies that are subexpressions of other dependencies. e.g. if a + * dependency is tracked in the temporaries sidemap, it can be added at + * site-of-use + */ + isDeferredDependency( + instr: + | {kind: HIRValue.Instruction; value: Instruction} + | {kind: HIRValue.Terminal; value: Terminal}, + ): boolean { + return ( + this.#processedInstrsInOptional.has(instr.value) || + (instr.kind === HIRValue.Instruction && + this.#temporaries.has(instr.value.lvalue.identifier.id)) + ); + } +} +enum HIRValue { + Instruction = 1, + Terminal, } function handleInstruction(instr: Instruction, context: Context): void { const {id, value, lvalue} = instr; - if (value.kind === 'LoadLocal') { - if ( - value.place.identifier.name === null || - lvalue.identifier.name !== null || - context.isUsedOutsideDeclaringScope(lvalue) - ) { - context.visitOperand(value.place); - } - } else if (value.kind === 'PropertyLoad') { - if (context.isUsedOutsideDeclaringScope(lvalue)) { - context.visitProperty(value.object, value.property, false); - } + context.declare(lvalue.identifier, { + id, + scope: context.currentScope, + }); + if ( + context.isDeferredDependency({kind: HIRValue.Instruction, value: instr}) + ) { + return; + } + if (value.kind === 'PropertyLoad') { + context.visitProperty(value.object, value.property, false); } else if (value.kind === 'StoreLocal') { context.visitOperand(value.value); if (value.lvalue.kind === InstructionKind.Reassign) { @@ -632,11 +688,6 @@ function handleInstruction(instr: Instruction, context: Context): void { context.visitOperand(operand); } } - - context.declare(lvalue.identifier, { - id, - scope: context.currentScope, - }); } function collectDependencies( @@ -645,7 +696,11 @@ function collectDependencies( temporaries: ReadonlyMap, processedInstrsInOptional: ReadonlySet, ): Map> { - const context = new Context(usedOutsideDeclaringScope, temporaries); + const context = new Context( + usedOutsideDeclaringScope, + temporaries, + processedInstrsInOptional, + ); for (const param of fn.params) { if (param.kind === 'Identifier') { @@ -694,16 +749,26 @@ function collectDependencies( /** * Recursively visit the inner function to extract dependencies there */ - const wasInInnerFn = context.inInnerFn; - context.inInnerFn = true; - handleFunction(instr.value.loweredFunc.func); - context.inInnerFn = wasInInnerFn; - } else if (!processedInstrsInOptional.has(instr)) { + const innerFn = instr.value.loweredFunc.func; + context.enterInnerFn( + instr as + | TInstruction + | TInstruction, + () => { + handleFunction(innerFn); + }, + ); + } else { handleInstruction(instr, context); } } - if (!processedInstrsInOptional.has(block.terminal)) { + if ( + !context.isDeferredDependency({ + kind: HIRValue.Terminal, + value: block.terminal, + }) + ) { for (const place of eachTerminalOperand(block.terminal)) { context.visitOperand(place); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts index 684acaf298388..75bd8f6811ffb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -19,7 +19,6 @@ import { import {deadCodeElimination} from '../Optimization'; import {inferReactiveScopeVariables} from '../ReactiveScopes'; import {rewriteInstructionKindsBasedOnReassignment} from '../SSA'; -import {logHIRFunction} from '../Utils/logger'; import {inferMutableContextVariables} from './InferMutableContextVariables'; import {inferMutableRanges} from './InferMutableRanges'; import inferReferenceEffects from './InferReferenceEffects'; @@ -112,7 +111,11 @@ function lower(func: HIRFunction): void { rewriteInstructionKindsBasedOnReassignment(func); inferReactiveScopeVariables(func); inferMutableContextVariables(func); - logHIRFunction('AnalyseFunction (inner)', func); + func.env.logger?.debugLogIRs?.({ + kind: 'hir', + name: 'AnalyseFunction (inner)', + value: func, + }); } function infer( diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OptimizePropsMethodCalls.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OptimizePropsMethodCalls.ts new file mode 100644 index 0000000000000..ab686ca219315 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OptimizePropsMethodCalls.ts @@ -0,0 +1,52 @@ +/** + * 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. + */ + +import {HIRFunction, isPropsType} from '../HIR'; + +/** + * Converts method calls into regular calls where the receiver is the props object: + * + * Example: + * + * ``` + * // INPUT + * props.foo(); + * + * // OUTPUT + * const t0 = props.foo; + * t0(); + * ``` + * + * Counter example: + * + * Here the receiver is `props.foo`, not the props object, so we don't rewrite it: + * + * // INPUT + * props.foo.bar(); + * + * // OUTPUT + * props.foo.bar(); + * ``` + */ +export function optimizePropsMethodCalls(fn: HIRFunction): void { + for (const [, block] of fn.body.blocks) { + for (let i = 0; i < block.instructions.length; i++) { + const instr = block.instructions[i]!; + if ( + instr.value.kind === 'MethodCall' && + isPropsType(instr.value.receiver.identifier) + ) { + instr.value = { + kind: 'CallExpression', + callee: instr.value.property, + args: instr.value.args, + loc: instr.value.loc, + }; + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts index 098139b150d5a..1108422f070d7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -25,7 +25,6 @@ import { eachPatternOperand, } from '../HIR/visitors'; import DisjointSet from '../Utils/DisjointSet'; -import {logHIRFunction} from '../Utils/logger'; import {assertExhaustive} from '../Utils/utils'; /* @@ -156,7 +155,11 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void { scope.range.end > maxInstruction + 1 ) { // Make it easier to debug why the error occurred - logHIRFunction('InferReactiveScopeVariables (invalid scope)', fn); + fn.env.logger?.debugLogIRs?.({ + kind: 'hir', + name: 'InferReactiveScopeVariables (invalid scope)', + value: fn, + }); CompilerError.invariant(false, { reason: `Invalid mutable range for scope`, loc: GeneratedSource, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/logger.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/logger.ts deleted file mode 100644 index fa43a8befeb83..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/logger.ts +++ /dev/null @@ -1,110 +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. - */ - -import generate from '@babel/generator'; -import * as t from '@babel/types'; -import chalk from 'chalk'; -import {HIR, HIRFunction, ReactiveFunction} from '../HIR/HIR'; -import {printFunctionWithOutlined, printHIR} from '../HIR/PrintHIR'; -import {CodegenFunction} from '../ReactiveScopes'; -import {printReactiveFunctionWithOutlined} from '../ReactiveScopes/PrintReactiveFunction'; - -let ENABLED: boolean = false; - -let lastLogged: string; - -export function toggleLogging(enabled: boolean): void { - ENABLED = enabled; -} - -export function logDebug(step: string, value: string): void { - if (ENABLED) { - process.stdout.write(`${chalk.green(step)}:\n${value}\n\n`); - } -} - -export function logHIR(step: string, ir: HIR): void { - if (ENABLED) { - const printed = printHIR(ir); - if (printed !== lastLogged) { - lastLogged = printed; - process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`); - } else { - process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`); - } - } -} - -export function logCodegenFunction(step: string, fn: CodegenFunction): void { - if (ENABLED) { - let printed: string | null = null; - try { - const node = t.functionDeclaration( - fn.id, - fn.params, - fn.body, - fn.generator, - fn.async, - ); - const ast = generate(node); - printed = ast.code; - } catch (e) { - let errMsg: string; - if ( - typeof e === 'object' && - e != null && - 'message' in e && - typeof e.message === 'string' - ) { - errMsg = e.message.toString(); - } else { - errMsg = '[empty]'; - } - console.log('Error formatting AST: ' + errMsg); - } - if (printed === null) { - return; - } - if (printed !== lastLogged) { - lastLogged = printed; - process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`); - } else { - process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`); - } - } -} - -export function logHIRFunction(step: string, fn: HIRFunction): void { - if (ENABLED) { - const printed = printFunctionWithOutlined(fn); - if (printed !== lastLogged) { - lastLogged = printed; - process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`); - } else { - process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`); - } - } -} - -export function logReactiveFunction(step: string, fn: ReactiveFunction): void { - if (ENABLED) { - const printed = printReactiveFunctionWithOutlined(fn); - if (printed !== lastLogged) { - lastLogged = printed; - process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`); - } else { - process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`); - } - } -} - -export function log(fn: () => string): void { - if (ENABLED) { - const message = fn(); - process.stdout.write(message.trim() + '\n\n'); - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md new file mode 100644 index 0000000000000..b8c7f8d4225f7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md @@ -0,0 +1,129 @@ + +## Input + +```javascript +import {makeArray, mutate} from 'shared-runtime'; + +/** + * Bug repro: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe"}} + * Forget: + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}} + * + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly + * aliasing `y` via `[y]`, we make an opaque call. + * + * Note that the bug here is that we don't infer that `a = makeArray(y)` + * potentially captures a context variable into a local variable. As a result, + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're + * currently inferring that this lambda captures `y` (for a potential later + * mutation) and simply reads `x`. + * + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not + * used when we analyze CallExpressions. + */ +function Component({foo, bar}: {foo: number; bar: number}) { + let x = {foo}; + let y: {bar: number; x?: {foo: number}} = {bar}; + const f0 = function () { + let a = makeArray(y); // a = [y] + let b = x; + // this writes y.x = x + a[0].x = b; + }; + f0(); + mutate(y.x); + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{foo: 3, bar: 4}], + sequentialRenders: [ + {foo: 3, bar: 4}, + {foo: 3, bar: 5}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray, mutate } from "shared-runtime"; + +/** + * Bug repro: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe"}} + * Forget: + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}} + * + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly + * aliasing `y` via `[y]`, we make an opaque call. + * + * Note that the bug here is that we don't infer that `a = makeArray(y)` + * potentially captures a context variable into a local variable. As a result, + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're + * currently inferring that this lambda captures `y` (for a potential later + * mutation) and simply reads `x`. + * + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not + * used when we analyze CallExpressions. + */ +function Component(t0) { + const $ = _c(5); + const { foo, bar } = t0; + let t1; + if ($[0] !== foo) { + t1 = { foo }; + $[0] = foo; + $[1] = t1; + } else { + t1 = $[1]; + } + const x = t1; + let y; + if ($[2] !== bar || $[3] !== x) { + y = { bar }; + const f0 = function () { + const a = makeArray(y); + const b = x; + + a[0].x = b; + }; + + f0(); + mutate(y.x); + $[2] = bar; + $[3] = x; + $[4] = y; + } else { + y = $[4]; + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ foo: 3, bar: 4 }], + sequentialRenders: [ + { foo: 3, bar: 4 }, + { foo: 3, bar: 5 }, + ], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts new file mode 100644 index 0000000000000..ca7076fda4019 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts @@ -0,0 +1,48 @@ +import {makeArray, mutate} from 'shared-runtime'; + +/** + * Bug repro: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe"}} + * Forget: + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}} + * + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly + * aliasing `y` via `[y]`, we make an opaque call. + * + * Note that the bug here is that we don't infer that `a = makeArray(y)` + * potentially captures a context variable into a local variable. As a result, + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're + * currently inferring that this lambda captures `y` (for a potential later + * mutation) and simply reads `x`. + * + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not + * used when we analyze CallExpressions. + */ +function Component({foo, bar}: {foo: number; bar: number}) { + let x = {foo}; + let y: {bar: number; x?: {foo: number}} = {bar}; + const f0 = function () { + let a = makeArray(y); // a = [y] + let b = x; + // this writes y.x = x + a[0].x = b; + }; + f0(); + mutate(y.x); + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{foo: 3, bar: 4}], + sequentialRenders: [ + {foo: 3, bar: 4}, + {foo: 3, bar: 5}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md index 2b0031b117be2..f8712ed7289a9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md @@ -58,18 +58,16 @@ function Foo(t0) { bar = $[1]; result = $[2]; } - - const t1 = bar; - let t2; - if ($[3] !== result || $[4] !== t1) { - t2 = ; - $[3] = result; - $[4] = t1; - $[5] = t2; + let t1; + if ($[3] !== bar || $[4] !== result) { + t1 = ; + $[3] = bar; + $[4] = result; + $[5] = t1; } else { - t2 = $[5]; + t1 = $[5]; } - return t2; + return t1; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md index 7febb3fecb6e7..1268cbcfdc3d3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md @@ -43,16 +43,15 @@ function Component(props) { } else { x = $[1]; } - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = { x: t0 }; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = { x }; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md index 26b56ea2a4f4d..769e4871f4ad8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md @@ -42,16 +42,15 @@ function Component(props) { } else { x = $[1]; } - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 =
{t0}
; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 =
{x}
; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md index 5ffa73389ffd0..e66ef2df13d5f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md @@ -43,16 +43,15 @@ function Component(props) { } else { x = $[1]; } - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = { x: t0 }; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = { x }; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md index 2c495d8223d0b..66799c5c4720b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md @@ -42,16 +42,15 @@ function Component(props) { } else { x = $[1]; } - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = { x: t0 }; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = { x }; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.expect.md index b84229156bc14..108c6725f7e8a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.expect.md @@ -3,7 +3,7 @@ ```javascript // @enableJsxOutlining -function Component(arr) { +function Component({arr}) { const x = useX(); return arr.map(i => { <> @@ -49,12 +49,13 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; // @enableJsxOutlining -function Component(arr) { +function Component(t0) { const $ = _c(3); + const { arr } = t0; const x = useX(); - let t0; + let t1; if ($[0] !== arr || $[1] !== x) { - t0 = arr.map((i) => { + t1 = arr.map((i) => { arr.map((i_0, id) => { const T0 = _temp; const child = ; @@ -65,11 +66,11 @@ function Component(arr) { }); $[0] = arr; $[1] = x; - $[2] = t0; + $[2] = t1; } else { - t0 = $[2]; + t1 = $[2]; } - return t0; + return t1; } function _temp(t0) { const $ = _c(5); @@ -140,4 +141,4 @@ export const FIXTURE_ENTRYPOINT = { ``` ### Eval output -(kind: exception) arr.map is not a function \ No newline at end of file +(kind: ok) [null,null] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.js index b7a82cd2a4be2..96a4e7bb2484b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.js @@ -1,5 +1,5 @@ // @enableJsxOutlining -function Component(arr) { +function Component({arr}) { const x = useX(); return arr.map(i => { <> diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lambda-mutated-non-reactive-to-reactive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lambda-mutated-non-reactive-to-reactive.expect.md index dfe941282e2a3..d34db46d6aa28 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lambda-mutated-non-reactive-to-reactive.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lambda-mutated-non-reactive-to-reactive.expect.md @@ -33,17 +33,15 @@ function f(a) { } else { x = $[1]; } - - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 =
; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 =
; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md deleted file mode 100644 index ae44f27912293..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md +++ /dev/null @@ -1,53 +0,0 @@ - -## Input - -```javascript -// @validatePreserveExistingMemoizationGuarantees -import {useCallback} from 'react'; -import {Stringify} from 'shared-runtime'; - -/** - * TODO: we're currently bailing out because `contextVar` is a context variable - * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad - * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted - * `LoadContext` and `PropertyLoad` instructions into the outer function, which - * we took as eligible dependencies. - * - * One solution is to simply record `LoadContext` identifiers into the - * temporaries sidemap when the instruction occurs *after* the context - * variable's mutable range. - */ -function Foo(props) { - let contextVar; - if (props.cond) { - contextVar = {val: 2}; - } else { - contextVar = {}; - } - - const cb = useCallback(() => [contextVar.val], [contextVar.val]); - - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{cond: true}], -}; - -``` - - -## Error - -``` - 22 | } - 23 | -> 24 | const cb = useCallback(() => [contextVar.val], [contextVar.val]); - | ^^^^^^^^^^^^^^^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (24:24) - 25 | - 26 | return ; - 27 | } -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md new file mode 100644 index 0000000000000..a1cbe89a88340 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md @@ -0,0 +1,101 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees +import {useCallback} from 'react'; +import {Stringify} from 'shared-runtime'; + +/** + * TODO: we're currently bailing out because `contextVar` is a context variable + * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad + * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted + * `LoadContext` and `PropertyLoad` instructions into the outer function, which + * we took as eligible dependencies. + * + * One solution is to simply record `LoadContext` identifiers into the + * temporaries sidemap when the instruction occurs *after* the context + * variable's mutable range. + */ +function Foo(props) { + let contextVar; + if (props.cond) { + contextVar = {val: 2}; + } else { + contextVar = {}; + } + + const cb = useCallback(() => [contextVar.val], [contextVar.val]); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{cond: true}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees +import { useCallback } from "react"; +import { Stringify } from "shared-runtime"; + +/** + * TODO: we're currently bailing out because `contextVar` is a context variable + * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad + * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted + * `LoadContext` and `PropertyLoad` instructions into the outer function, which + * we took as eligible dependencies. + * + * One solution is to simply record `LoadContext` identifiers into the + * temporaries sidemap when the instruction occurs *after* the context + * variable's mutable range. + */ +function Foo(props) { + const $ = _c(6); + let contextVar; + if ($[0] !== props.cond) { + if (props.cond) { + contextVar = { val: 2 }; + } else { + contextVar = {}; + } + $[0] = props.cond; + $[1] = contextVar; + } else { + contextVar = $[1]; + } + let t0; + if ($[2] !== contextVar.val) { + t0 = () => [contextVar.val]; + $[2] = contextVar.val; + $[3] = t0; + } else { + t0 = $[3]; + } + contextVar; + const cb = t0; + let t1; + if ($[4] !== cb) { + t1 = ; + $[4] = cb; + $[5] = t1; + } else { + t1 = $[5]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: true }], +}; + +``` + +### Eval output +(kind: ok)
{"cb":{"kind":"Function","result":[2]},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md index dc1a87fe5113c..e8a3e2d627c59 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md @@ -44,16 +44,15 @@ function useFoo(arr1, arr2) { y = $[2]; } let t0; - const t1 = y; - let t2; - if ($[3] !== t1) { - t2 = { y: t1 }; - $[3] = t1; - $[4] = t2; + let t1; + if ($[3] !== y) { + t1 = { y }; + $[3] = y; + $[4] = t1; } else { - t2 = $[4]; + t1 = $[4]; } - t0 = t2; + t0 = t1; return t0; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-scope-missing-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-scope-missing-mutable-range.expect.md index 39f301432e51f..9d232d8e78169 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-scope-missing-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-scope-missing-mutable-range.expect.md @@ -36,17 +36,15 @@ function HomeDiscoStoreItemTileRating(props) { } else { count = $[1]; } - - const t0 = count; - let t1; - if ($[2] !== t0) { - t1 = {t0}; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== count) { + t0 = {count}; + $[2] = count; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/props-method-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/props-method-dependency.expect.md new file mode 100644 index 0000000000000..3297892ea2469 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/props-method-dependency.expect.md @@ -0,0 +1,78 @@ + +## Input + +```javascript +// @compilationMode(infer) +import {useMemo} from 'react'; +import {ValidateMemoization} from 'shared-runtime'; + +function Component(props) { + const x = useMemo(() => props.x(), [props.x]); + return ; +} + +const f = () => ['React']; +const g = () => ['Compiler']; +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: () => ['React']}], + sequentialRenders: [{x: f}, {x: g}, {x: g}, {x: f}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @compilationMode(infer) +import { useMemo } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +function Component(props) { + const $ = _c(7); + let t0; + let t1; + if ($[0] !== props.x) { + t1 = props.x(); + $[0] = props.x; + $[1] = t1; + } else { + t1 = $[1]; + } + t0 = t1; + const x = t0; + let t2; + if ($[2] !== props.x) { + t2 = [props.x]; + $[2] = props.x; + $[3] = t2; + } else { + t2 = $[3]; + } + let t3; + if ($[4] !== t2 || $[5] !== x) { + t3 = ; + $[4] = t2; + $[5] = x; + $[6] = t3; + } else { + t3 = $[6]; + } + return t3; +} + +const f = () => ["React"]; +const g = () => ["Compiler"]; +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ x: () => ["React"] }], + sequentialRenders: [{ x: f }, { x: g }, { x: g }, { x: f }], +}; + +``` + +### Eval output +(kind: ok)
{"inputs":["[[ function params=0 ]]"],"output":["React"]}
+
{"inputs":["[[ function params=0 ]]"],"output":["Compiler"]}
+
{"inputs":["[[ function params=0 ]]"],"output":["Compiler"]}
+
{"inputs":["[[ function params=0 ]]"],"output":["React"]}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/props-method-dependency.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/props-method-dependency.js new file mode 100644 index 0000000000000..4c2d322ad3399 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/props-method-dependency.js @@ -0,0 +1,16 @@ +// @compilationMode(infer) +import {useMemo} from 'react'; +import {ValidateMemoization} from 'shared-runtime'; + +function Component(props) { + const x = useMemo(() => props.x(), [props.x]); + return ; +} + +const f = () => ['React']; +const g = () => ['Compiler']; +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: () => ['React']}], + sequentialRenders: [{x: f}, {x: g}, {x: g}, {x: f}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md index 23cc7ee84607b..ceaa350012258 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md @@ -67,17 +67,15 @@ function Component(props) { } else { x = $[1]; } - - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = [t0]; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = [x]; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md new file mode 100644 index 0000000000000..d72f34b4fd8a9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md @@ -0,0 +1,130 @@ + +## Input + +```javascript +import {throwErrorWithMessage, ValidateMemoization} from 'shared-runtime'; + +/** + * Context variables are local variables that (1) have at least one reassignment + * and (2) are captured into a function expression. These have a known mutable + * range: from first declaration / assignment to the last direct or aliased, + * mutable reference. + * + * This fixture validates that forget can take granular dependencies on context + * variables when the reference to a context var happens *after* the end of its + * mutable range. + */ +function Component({cond, a}) { + let contextVar; + if (cond) { + contextVar = {val: a}; + } else { + contextVar = {}; + throwErrorWithMessage(''); + } + const cb = {cb: () => contextVar.val * 4}; + + /** + * manually specify input to avoid adding a `PropertyLoad` from contextVar, + * which might affect hoistable-objects analysis. + */ + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false, a: undefined}], + sequentialRenders: [ + {cond: true, a: 2}, + {cond: true, a: 2}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { throwErrorWithMessage, ValidateMemoization } from "shared-runtime"; + +/** + * Context variables are local variables that (1) have at least one reassignment + * and (2) are captured into a function expression. These have a known mutable + * range: from first declaration / assignment to the last direct or aliased, + * mutable reference. + * + * This fixture validates that forget can take granular dependencies on context + * variables when the reference to a context var happens *after* the end of its + * mutable range. + */ +function Component(t0) { + const $ = _c(10); + const { cond, a } = t0; + let contextVar; + if ($[0] !== a || $[1] !== cond) { + if (cond) { + contextVar = { val: a }; + } else { + contextVar = {}; + throwErrorWithMessage(""); + } + $[0] = a; + $[1] = cond; + $[2] = contextVar; + } else { + contextVar = $[2]; + } + let t1; + if ($[3] !== contextVar.val) { + t1 = { cb: () => contextVar.val * 4 }; + $[3] = contextVar.val; + $[4] = t1; + } else { + t1 = $[4]; + } + const cb = t1; + + const t2 = cond ? a : undefined; + let t3; + if ($[5] !== t2) { + t3 = [t2]; + $[5] = t2; + $[6] = t3; + } else { + t3 = $[6]; + } + let t4; + if ($[7] !== cb || $[8] !== t3) { + t4 = ( + + ); + $[7] = cb; + $[8] = t3; + $[9] = t4; + } else { + t4 = $[9]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: false, a: undefined }], + sequentialRenders: [ + { cond: true, a: 2 }, + { cond: true, a: 2 }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"inputs":[2],"output":{"cb":"[[ function params=0 ]]"}}
+
{"inputs":[2],"output":{"cb":"[[ function params=0 ]]"}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.js new file mode 100644 index 0000000000000..b9bdd67e2f504 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.js @@ -0,0 +1,43 @@ +import {throwErrorWithMessage, ValidateMemoization} from 'shared-runtime'; + +/** + * Context variables are local variables that (1) have at least one reassignment + * and (2) are captured into a function expression. These have a known mutable + * range: from first declaration / assignment to the last direct or aliased, + * mutable reference. + * + * This fixture validates that forget can take granular dependencies on context + * variables when the reference to a context var happens *after* the end of its + * mutable range. + */ +function Component({cond, a}) { + let contextVar; + if (cond) { + contextVar = {val: a}; + } else { + contextVar = {}; + throwErrorWithMessage(''); + } + const cb = {cb: () => contextVar.val * 4}; + + /** + * manually specify input to avoid adding a `PropertyLoad` from contextVar, + * which might affect hoistable-objects analysis. + */ + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false, a: undefined}], + sequentialRenders: [ + {cond: true, a: 2}, + {cond: true, a: 2}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md index d8e59c486a55b..b7c425ba5c027 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md @@ -35,17 +35,15 @@ function HomeDiscoStoreItemTileRating(props) { } else { count = $[1]; } - - const t0 = count; - let t1; - if ($[2] !== t0) { - t1 = {t0}; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== count) { + t0 = {count}; + $[2] = count; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.expect.md new file mode 100644 index 0000000000000..e491eb6c69d3c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +import {arrayPush} from 'shared-runtime'; + +function useFoo({a, b}) { + const obj = {a}; + arrayPush(Object.keys(obj), b); + return obj; +} +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2, b: 3}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { arrayPush } from "shared-runtime"; + +function useFoo(t0) { + const $ = _c(2); + const { a, b } = t0; + let t1; + if ($[0] !== a) { + t1 = { a }; + $[0] = a; + $[1] = t1; + } else { + t1 = $[1]; + } + const obj = t1; + arrayPush(Object.keys(obj), b); + return obj; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ a: 2, b: 3 }], +}; + +``` + +### Eval output +(kind: ok) {"a":2} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.ts new file mode 100644 index 0000000000000..9dbaac79c6b5e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.ts @@ -0,0 +1,11 @@ +import {arrayPush} from 'shared-runtime'; + +function useFoo({a, b}) { + const obj = {a}; + arrayPush(Object.keys(obj), b); + return obj; +} +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2, b: 3}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.expect.md new file mode 100644 index 0000000000000..69c1b9bbbbc73 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +// @compilationMode(all) +'use no memo'; + +function TestComponent({x}) { + 'use memo'; + return ; +} + +``` + +## Code + +```javascript +// @compilationMode(all) +"use no memo"; + +function TestComponent({ x }) { + "use memo"; + return ; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.js new file mode 100644 index 0000000000000..9b314e1f99d53 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.js @@ -0,0 +1,7 @@ +// @compilationMode(all) +'use no memo'; + +function TestComponent({x}) { + 'use memo'; + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-operator-conditional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-operator-conditional.expect.md index d94a5e7e375b3..e335273026791 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-operator-conditional.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-operator-conditional.expect.md @@ -88,36 +88,34 @@ function Inner(props) { input; input; let t0; - const t1 = input; - let t2; - if ($[0] !== t1) { - t2 = [t1]; - $[0] = t1; - $[1] = t2; + let t1; + if ($[0] !== input) { + t1 = [input]; + $[0] = input; + $[1] = t1; } else { - t2 = $[1]; + t1 = $[1]; } - t0 = t2; + t0 = t1; const output = t0; - const t3 = input; - let t4; - if ($[2] !== t3) { - t4 = [t3]; - $[2] = t3; - $[3] = t4; + let t2; + if ($[2] !== input) { + t2 = [input]; + $[2] = input; + $[3] = t2; } else { - t4 = $[3]; + t2 = $[3]; } - let t5; - if ($[4] !== output || $[5] !== t4) { - t5 = ; + let t3; + if ($[4] !== output || $[5] !== t2) { + t3 = ; $[4] = output; - $[5] = t4; - $[6] = t5; + $[5] = t2; + $[6] = t3; } else { - t5 = $[6]; + t3 = $[6]; } - return t5; + return t3; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts index d634bd235f190..dc4d5d25a47e2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts @@ -25,6 +25,7 @@ describe('parseConfigPragmaForTests()', () => { enableUseTypeAnnotations: true, validateNoSetStateInPassiveEffects: true, validateNoSetStateInRender: false, + enableResetCacheOnSourceFileChanges: false, }); }); }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/index.ts b/compiler/packages/babel-plugin-react-compiler/src/index.ts index 150df26e45818..188c244d9ef2c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/index.ts @@ -17,8 +17,6 @@ export { compileFn as compile, compileProgram, parsePluginOptions, - run, - runPlayground, OPT_OUT_DIRECTIVES, OPT_IN_DIRECTIVES, findDirectiveEnablingMemoization, diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 36b3e92f9636b..361070739630c 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -479,6 +479,7 @@ const skipFilter = new Set([ // bugs 'fbt/bug-fbt-plural-multiple-function-calls', 'fbt/bug-fbt-plural-multiple-mixed-call-tag', + `bug-capturing-func-maybealias-captured-mutate`, 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr', 'bug-invalid-hoisting-functionexpr', 'bug-aliased-capture-aliased-mutate', diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 1cb8fe48b9451..a95c61450d840 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -19,6 +19,7 @@ import type { PanicThresholdOptions, PluginOptions, CompilerReactTarget, + CompilerPipelineValue, } from 'babel-plugin-react-compiler/src/Entrypoint'; import type {Effect, ValueKind} from 'babel-plugin-react-compiler/src/HIR'; import type { @@ -45,6 +46,7 @@ export function parseLanguage(source: string): 'flow' | 'typescript' { function makePluginOptions( firstLine: string, parseConfigPragmaFn: typeof ParseConfigPragma, + debugIRLogger: (value: CompilerPipelineValue) => void, EffectEnum: typeof Effect, ValueKindEnum: typeof ValueKind, ): [PluginOptions, Array<{filename: string | null; event: LoggerEvent}>] { @@ -56,6 +58,7 @@ function makePluginOptions( let validatePreserveExistingMemoizationGuarantees = false; let customMacros: null | Array = null; let validateBlocklistedImports = null; + let enableFire = false; let target: CompilerReactTarget = '19'; if (firstLine.indexOf('@compilationMode(annotation)') !== -1) { @@ -127,6 +130,10 @@ function makePluginOptions( validatePreserveExistingMemoizationGuarantees = true; } + if (firstLine.includes('@enableFire')) { + enableFire = true; + } + const hookPatternMatch = /@hookPattern:"([^"]+)"/.exec(firstLine); if ( hookPatternMatch && @@ -182,15 +189,15 @@ function makePluginOptions( .filter(s => s.length > 0); } - let logs: Array<{filename: string | null; event: LoggerEvent}> = []; - let logger: Logger | null = null; - if (firstLine.includes('@logger')) { - logger = { - logEvent(filename: string | null, event: LoggerEvent): void { - logs.push({filename, event}); - }, - }; - } + const logs: Array<{filename: string | null; event: LoggerEvent}> = []; + const logger: Logger = { + logEvent: firstLine.includes('@logger') + ? (filename, event) => { + logs.push({filename, event}); + } + : () => {}, + debugLogIRs: debugIRLogger, + }; const config = parseConfigPragmaFn(firstLine); const options = { @@ -205,6 +212,7 @@ function makePluginOptions( hookPattern, validatePreserveExistingMemoizationGuarantees, validateBlocklistedImports, + enableFire, }, compilationMode, logger, @@ -338,6 +346,7 @@ export async function transformFixtureInput( parseConfigPragmaFn: typeof ParseConfigPragma, plugin: BabelCore.PluginObj, includeEvaluator: boolean, + debugIRLogger: (value: CompilerPipelineValue) => void, EffectEnum: typeof Effect, ValueKindEnum: typeof ValueKind, ): Promise<{kind: 'ok'; value: TransformResult} | {kind: 'err'; msg: string}> { @@ -365,6 +374,7 @@ export async function transformFixtureInput( const [options, logs] = makePluginOptions( firstLine, parseConfigPragmaFn, + debugIRLogger, EffectEnum, ValueKindEnum, ); diff --git a/compiler/packages/snap/src/constants.ts b/compiler/packages/snap/src/constants.ts index abee06c55be8a..ad77441b532df 100644 --- a/compiler/packages/snap/src/constants.ts +++ b/compiler/packages/snap/src/constants.ts @@ -18,11 +18,17 @@ export const COMPILER_PATH = path.join( 'BabelPlugin.js', ); export const COMPILER_INDEX_PATH = path.join(process.cwd(), 'dist', 'index'); -export const LOGGER_PATH = path.join( +export const PRINT_HIR_PATH = path.join( process.cwd(), 'dist', - 'Utils', - 'logger.js', + 'HIR', + 'PrintHIR.js', +); +export const PRINT_REACTIVE_IR_PATH = path.join( + process.cwd(), + 'dist', + 'ReactiveScopes', + 'PrintReactiveFunction.js', ); export const PARSE_CONFIG_PRAGMA_PATH = path.join( process.cwd(), diff --git a/compiler/packages/snap/src/runner-worker.ts b/compiler/packages/snap/src/runner-worker.ts index f05757d3df68d..ea87cd1e91d16 100644 --- a/compiler/packages/snap/src/runner-worker.ts +++ b/compiler/packages/snap/src/runner-worker.ts @@ -8,16 +8,21 @@ import {codeFrameColumns} from '@babel/code-frame'; import type {PluginObj} from '@babel/core'; import type {parseConfigPragmaForTests as ParseConfigPragma} from 'babel-plugin-react-compiler/src/HIR/Environment'; +import type {printFunctionWithOutlined as PrintFunctionWithOutlined} from 'babel-plugin-react-compiler/src/HIR/PrintHIR'; +import type {printReactiveFunctionWithOutlined as PrintReactiveFunctionWithOutlined} from 'babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction'; import {TransformResult, transformFixtureInput} from './compiler'; import { COMPILER_PATH, COMPILER_INDEX_PATH, - LOGGER_PATH, PARSE_CONFIG_PRAGMA_PATH, + PRINT_HIR_PATH, + PRINT_REACTIVE_IR_PATH, } from './constants'; import {TestFixture, getBasename, isExpectError} from './fixture-utils'; import {TestResult, writeOutputToString} from './reporter'; import {runSprout} from './sprout'; +import {CompilerPipelineValue} from 'babel-plugin-react-compiler/src'; +import chalk from 'chalk'; const originalConsoleError = console.error; @@ -64,20 +69,56 @@ async function compile( const {Effect: EffectEnum, ValueKind: ValueKindEnum} = require( COMPILER_INDEX_PATH, ); - const {toggleLogging} = require(LOGGER_PATH); + const {printFunctionWithOutlined} = require(PRINT_HIR_PATH) as { + printFunctionWithOutlined: typeof PrintFunctionWithOutlined; + }; + const {printReactiveFunctionWithOutlined} = require( + PRINT_REACTIVE_IR_PATH, + ) as { + printReactiveFunctionWithOutlined: typeof PrintReactiveFunctionWithOutlined; + }; + + let lastLogged: string | null = null; + const debugIRLogger = shouldLog + ? (value: CompilerPipelineValue) => { + let printed: string; + switch (value.kind) { + case 'hir': + printed = printFunctionWithOutlined(value.value); + break; + case 'reactive': + printed = printReactiveFunctionWithOutlined(value.value); + break; + case 'debug': + printed = value.value; + break; + case 'ast': + // skip printing ast as we already write fixture output JS + printed = '(ast)'; + break; + } + + if (printed !== lastLogged) { + lastLogged = printed; + console.log(`${chalk.green(value.name)}:\n ${printed}\n`); + } else { + console.log(`${chalk.blue(value.name)}: (no change)\n`); + } + } + : () => {}; const {parseConfigPragmaForTests} = require(PARSE_CONFIG_PRAGMA_PATH) as { parseConfigPragmaForTests: typeof ParseConfigPragma; }; // only try logging if we filtered out all but one fixture, // since console log order is non-deterministic - toggleLogging(shouldLog); const result = await transformFixtureInput( input, fixturePath, parseConfigPragmaForTests, BabelPluginReactCompiler, includeEvaluator, + debugIRLogger, EffectEnum, ValueKindEnum, ); diff --git a/compiler/packages/snap/src/sprout/index.ts b/compiler/packages/snap/src/sprout/index.ts index 733be561c08a8..04748bed28f4f 100644 --- a/compiler/packages/snap/src/sprout/index.ts +++ b/compiler/packages/snap/src/sprout/index.ts @@ -32,7 +32,15 @@ export function runSprout( originalCode: string, forgetCode: string, ): SproutResult { - const forgetResult = doEval(forgetCode); + let forgetResult; + try { + (globalThis as any).__SNAP_EVALUATOR_MODE = 'forget'; + forgetResult = doEval(forgetCode); + } catch (e) { + throw e; + } finally { + (globalThis as any).__SNAP_EVALUATOR_MODE = undefined; + } if (forgetResult.kind === 'UnexpectedError') { return makeError('Unexpected error in Forget runner', forgetResult.value); } diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index 58815842cb03c..1b8648f4ff031 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -259,26 +259,35 @@ export function Throw() { export function ValidateMemoization({ inputs, - output, + output: rawOutput, + onlyCheckCompiled = false, }: { inputs: Array; output: any; + onlyCheckCompiled: boolean; }): React.ReactElement { 'use no forget'; + // Wrap rawOutput as it might be a function, which useState would invoke. + const output = {value: rawOutput}; const [previousInputs, setPreviousInputs] = React.useState(inputs); const [previousOutput, setPreviousOutput] = React.useState(output); if ( - inputs.length !== previousInputs.length || - inputs.some((item, i) => item !== previousInputs[i]) + onlyCheckCompiled && + (globalThis as any).__SNAP_EVALUATOR_MODE === 'forget' ) { - // Some input changed, we expect the output to change - setPreviousInputs(inputs); - setPreviousOutput(output); - } else if (output !== previousOutput) { - // Else output should be stable - throw new Error('Output identity changed but inputs did not'); + if ( + inputs.length !== previousInputs.length || + inputs.some((item, i) => item !== previousInputs[i]) + ) { + // Some input changed, we expect the output to change + setPreviousInputs(inputs); + setPreviousOutput(output); + } else if (output.value !== previousOutput.value) { + // Else output should be stable + throw new Error('Output identity changed but inputs did not'); + } } - return React.createElement(Stringify, {inputs, output}); + return React.createElement(Stringify, {inputs, output: rawOutput}); } export function createHookWrapper( diff --git a/fixtures/flight/src/App.js b/fixtures/flight/src/App.js index 49bfc9e05135c..69fbb5e0af97d 100644 --- a/fixtures/flight/src/App.js +++ b/fixtures/flight/src/App.js @@ -27,7 +27,8 @@ function Foo({children}) { return
{children}
; } -function Bar({children}) { +async function Bar({children}) { + await new Promise(resolve => setTimeout(() => resolve('deferred text'), 10)); return
{children}
; } @@ -81,7 +82,7 @@ export default async function App({prerender}) { {dedupedChild} - {dedupedChild} + {Promise.resolve([dedupedChild])} diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 4d5204ab00220..8e00df7c1c2e9 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -43,9 +43,7 @@ import type {Postpone} from 'react/src/ReactPostpone'; import type {TemporaryReferenceSet} from './ReactFlightTemporaryReferences'; import { - enableBinaryFlight, enablePostpone, - enableFlightReadableStream, enableOwnerStacks, enableServerComponentLogs, enableProfilerTimer, @@ -71,7 +69,11 @@ import {createBoundServerReference} from './ReactFlightReplyClient'; import {readTemporaryReference} from './ReactFlightTemporaryReferences'; -import {logComponentRender} from './ReactFlightPerformanceTrack'; +import { + markAllTracksInOrder, + logComponentRender, + logDedupedComponentRender, +} from './ReactFlightPerformanceTrack'; import { REACT_LAZY_TYPE, @@ -127,7 +129,9 @@ export type JSONValue = | $ReadOnlyArray; type ProfilingResult = { + track: number, endTime: number, + component: null | ReactComponentInfo, }; const ROW_ID = 0; @@ -407,14 +411,12 @@ function wakeChunkIfInitialized( function triggerErrorOnChunk(chunk: SomeChunk, error: mixed): void { if (chunk.status !== PENDING && chunk.status !== BLOCKED) { - if (enableFlightReadableStream) { - // If we get more data to an already resolved ID, we assume that it's - // a stream chunk since any other row shouldn't have more than one entry. - const streamChunk: InitializedStreamChunk = (chunk: any); - const controller = streamChunk.reason; - // $FlowFixMe[incompatible-call]: The error method should accept mixed. - controller.error(error); - } + // If we get more data to an already resolved ID, we assume that it's + // a stream chunk since any other row shouldn't have more than one entry. + const streamChunk: InitializedStreamChunk = (chunk: any); + const controller = streamChunk.reason; + // $FlowFixMe[incompatible-call]: The error method should accept mixed. + controller.error(error); return; } const listeners = chunk.reason; @@ -513,13 +515,11 @@ function resolveModelChunk( value: UninitializedModel, ): void { if (chunk.status !== PENDING) { - if (enableFlightReadableStream) { - // If we get more data to an already resolved ID, we assume that it's - // a stream chunk since any other row shouldn't have more than one entry. - const streamChunk: InitializedStreamChunk = (chunk: any); - const controller = streamChunk.reason; - controller.enqueueModel(value); - } + // If we get more data to an already resolved ID, we assume that it's + // a stream chunk since any other row shouldn't have more than one entry. + const streamChunk: InitializedStreamChunk = (chunk: any); + const controller = streamChunk.reason; + controller.enqueueModel(value); return; } const resolveListeners = chunk.value; @@ -648,7 +648,14 @@ export function reportGlobalError(response: Response, error: Error): void { } }); if (enableProfilerTimer && enableComponentPerformanceTrack) { - flushComponentPerformance(getChunk(response, 0)); + markAllTracksInOrder(); + flushComponentPerformance( + response, + getChunk(response, 0), + 0, + -Infinity, + -Infinity, + ); } } @@ -1461,11 +1468,8 @@ function parseModelString( } case 'B': { // Blob - if (enableBinaryFlight) { - const ref = value.slice(2); - return getOutlinedModel(response, ref, parentObject, key, createBlob); - } - return undefined; + const ref = value.slice(2); + return getOutlinedModel(response, ref, parentObject, key, createBlob); } case 'K': { // FormData @@ -1722,16 +1726,14 @@ function resolveModel( function resolveText(response: Response, id: number, text: string): void { const chunks = response._chunks; - if (enableFlightReadableStream) { - const chunk = chunks.get(id); - if (chunk && chunk.status !== PENDING) { - // If we get more data to an already resolved ID, we assume that it's - // a stream chunk since any other row shouldn't have more than one entry. - const streamChunk: InitializedStreamChunk = (chunk: any); - const controller = streamChunk.reason; - controller.enqueueValue(text); - return; - } + const chunk = chunks.get(id); + if (chunk && chunk.status !== PENDING) { + // If we get more data to an already resolved ID, we assume that it's + // a stream chunk since any other row shouldn't have more than one entry. + const streamChunk: InitializedStreamChunk = (chunk: any); + const controller = streamChunk.reason; + controller.enqueueValue(text); + return; } chunks.set(id, createInitializedTextChunk(response, text)); } @@ -1742,16 +1744,14 @@ function resolveBuffer( buffer: $ArrayBufferView | ArrayBuffer, ): void { const chunks = response._chunks; - if (enableFlightReadableStream) { - const chunk = chunks.get(id); - if (chunk && chunk.status !== PENDING) { - // If we get more data to an already resolved ID, we assume that it's - // a stream chunk since any other row shouldn't have more than one entry. - const streamChunk: InitializedStreamChunk = (chunk: any); - const controller = streamChunk.reason; - controller.enqueueValue(buffer); - return; - } + const chunk = chunks.get(id); + if (chunk && chunk.status !== PENDING) { + // If we get more data to an already resolved ID, we assume that it's + // a stream chunk since any other row shouldn't have more than one entry. + const streamChunk: InitializedStreamChunk = (chunk: any); + const controller = streamChunk.reason; + controller.enqueueValue(buffer); + return; } chunks.set(id, createInitializedBufferChunk(response, buffer)); } @@ -2753,9 +2753,18 @@ function resolveTypedArray( resolveBuffer(response, id, view); } -function flushComponentPerformance(root: SomeChunk): number { +function flushComponentPerformance( + response: Response, + root: SomeChunk, + trackIdx: number, // Next available track + trackTime: number, // The time after which it is available, + parentEndTime: number, +): ProfilingResult { if (!enableProfilerTimer || !enableComponentPerformanceTrack) { - return 0; + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'flushComponentPerformance should never be called in production mode. This is a bug in React.', + ); } // Write performance.measure() entries for Server Components in tree order. // This must be done at the end to collect the end time from the whole tree. @@ -2766,7 +2775,25 @@ function flushComponentPerformance(root: SomeChunk): number { // chunk in two places. We should extend the current end time as if it was // rendered as part of this tree. const previousResult: ProfilingResult = root._children; - return previousResult.endTime; + const previousEndTime = previousResult.endTime; + if ( + parentEndTime > -Infinity && + parentEndTime < previousEndTime && + previousResult.component !== null + ) { + // Log a placeholder for the deduped value under this child starting + // from the end of the self time of the parent and spanning until the + // the deduped end. + logDedupedComponentRender( + previousResult.component, + trackIdx, + parentEndTime, + previousEndTime, + ); + } + // Since we didn't bump the track this time, we just return the same track. + previousResult.track = trackIdx; + return previousResult; } const children = root._children; if (root.status === RESOLVED_MODEL) { @@ -2775,16 +2802,66 @@ function flushComponentPerformance(root: SomeChunk): number { // the performance characteristics of the app by profiling. initializeModelChunk(root); } - const result: ProfilingResult = {endTime: -Infinity}; + + // First find the start time of the first component to know if it was running + // in parallel with the previous. + const debugInfo = root._debugInfo; + if (debugInfo) { + for (let i = 1; i < debugInfo.length; i++) { + const info = debugInfo[i]; + if (typeof info.name === 'string') { + // $FlowFixMe: Refined. + const startTimeInfo = debugInfo[i - 1]; + if (typeof startTimeInfo.time === 'number') { + const startTime = startTimeInfo.time; + if (startTime < trackTime) { + // The start time of this component is before the end time of the previous + // component on this track so we need to bump the next one to a parallel track. + trackIdx++; + } + trackTime = startTime; + break; + } + } + } + for (let i = debugInfo.length - 1; i >= 0; i--) { + const info = debugInfo[i]; + if (typeof info.time === 'number') { + if (info.time > parentEndTime) { + parentEndTime = info.time; + } + } + } + } + + const result: ProfilingResult = { + track: trackIdx, + endTime: -Infinity, + component: null, + }; root._children = result; let childrenEndTime = -Infinity; + let childTrackIdx = trackIdx; + let childTrackTime = trackTime; for (let i = 0; i < children.length; i++) { - const childEndTime = flushComponentPerformance(children[i]); + const childResult = flushComponentPerformance( + response, + children[i], + childTrackIdx, + childTrackTime, + parentEndTime, + ); + if (childResult.component !== null) { + result.component = childResult.component; + } + childTrackIdx = childResult.track; + const childEndTime = childResult.endTime; + childTrackTime = childEndTime; if (childEndTime > childrenEndTime) { childrenEndTime = childEndTime; } } - const debugInfo = root._debugInfo; + if (debugInfo) { let endTime = 0; for (let i = debugInfo.length - 1; i >= 0; i--) { @@ -2803,15 +2880,20 @@ function flushComponentPerformance(root: SomeChunk): number { const startTime = startTimeInfo.time; logComponentRender( componentInfo, + trackIdx, startTime, endTime, childrenEndTime, + response._rootEnvironmentName, ); + // Track the root most component of the result for deduping logging. + result.component = componentInfo; } } } } - return (result.endTime = childrenEndTime); + result.endTime = childrenEndTime; + return result; } function processFullBinaryRow( @@ -2821,53 +2903,51 @@ function processFullBinaryRow( buffer: Array, chunk: Uint8Array, ): void { - if (enableBinaryFlight) { - switch (tag) { - case 65 /* "A" */: - // We must always clone to extract it into a separate buffer instead of just a view. - resolveBuffer(response, id, mergeBuffer(buffer, chunk).buffer); - return; - case 79 /* "O" */: - resolveTypedArray(response, id, buffer, chunk, Int8Array, 1); - return; - case 111 /* "o" */: - resolveBuffer( - response, - id, - buffer.length === 0 ? chunk : mergeBuffer(buffer, chunk), - ); - return; - case 85 /* "U" */: - resolveTypedArray(response, id, buffer, chunk, Uint8ClampedArray, 1); - return; - case 83 /* "S" */: - resolveTypedArray(response, id, buffer, chunk, Int16Array, 2); - return; - case 115 /* "s" */: - resolveTypedArray(response, id, buffer, chunk, Uint16Array, 2); - return; - case 76 /* "L" */: - resolveTypedArray(response, id, buffer, chunk, Int32Array, 4); - return; - case 108 /* "l" */: - resolveTypedArray(response, id, buffer, chunk, Uint32Array, 4); - return; - case 71 /* "G" */: - resolveTypedArray(response, id, buffer, chunk, Float32Array, 4); - return; - case 103 /* "g" */: - resolveTypedArray(response, id, buffer, chunk, Float64Array, 8); - return; - case 77 /* "M" */: - resolveTypedArray(response, id, buffer, chunk, BigInt64Array, 8); - return; - case 109 /* "m" */: - resolveTypedArray(response, id, buffer, chunk, BigUint64Array, 8); - return; - case 86 /* "V" */: - resolveTypedArray(response, id, buffer, chunk, DataView, 1); - return; - } + switch (tag) { + case 65 /* "A" */: + // We must always clone to extract it into a separate buffer instead of just a view. + resolveBuffer(response, id, mergeBuffer(buffer, chunk).buffer); + return; + case 79 /* "O" */: + resolveTypedArray(response, id, buffer, chunk, Int8Array, 1); + return; + case 111 /* "o" */: + resolveBuffer( + response, + id, + buffer.length === 0 ? chunk : mergeBuffer(buffer, chunk), + ); + return; + case 85 /* "U" */: + resolveTypedArray(response, id, buffer, chunk, Uint8ClampedArray, 1); + return; + case 83 /* "S" */: + resolveTypedArray(response, id, buffer, chunk, Int16Array, 2); + return; + case 115 /* "s" */: + resolveTypedArray(response, id, buffer, chunk, Uint16Array, 2); + return; + case 76 /* "L" */: + resolveTypedArray(response, id, buffer, chunk, Int32Array, 4); + return; + case 108 /* "l" */: + resolveTypedArray(response, id, buffer, chunk, Uint32Array, 4); + return; + case 71 /* "G" */: + resolveTypedArray(response, id, buffer, chunk, Float32Array, 4); + return; + case 103 /* "g" */: + resolveTypedArray(response, id, buffer, chunk, Float64Array, 8); + return; + case 77 /* "M" */: + resolveTypedArray(response, id, buffer, chunk, BigInt64Array, 8); + return; + case 109 /* "m" */: + resolveTypedArray(response, id, buffer, chunk, BigUint64Array, 8); + return; + case 86 /* "V" */: + resolveTypedArray(response, id, buffer, chunk, DataView, 1); + return; } const stringDecoder = response._stringDecoder; @@ -2973,38 +3053,28 @@ function processFullStringRow( ); } case 82 /* "R" */: { - if (enableFlightReadableStream) { - startReadableStream(response, id, undefined); - return; - } + startReadableStream(response, id, undefined); + return; } // Fallthrough case 114 /* "r" */: { - if (enableFlightReadableStream) { - startReadableStream(response, id, 'bytes'); - return; - } + startReadableStream(response, id, 'bytes'); + return; } // Fallthrough case 88 /* "X" */: { - if (enableFlightReadableStream) { - startAsyncIterable(response, id, false); - return; - } + startAsyncIterable(response, id, false); + return; } // Fallthrough case 120 /* "x" */: { - if (enableFlightReadableStream) { - startAsyncIterable(response, id, true); - return; - } + startAsyncIterable(response, id, true); + return; } // Fallthrough case 67 /* "C" */: { - if (enableFlightReadableStream) { - stopStream(response, id, row); - return; - } + stopStream(response, id, row); + return; } // Fallthrough case 80 /* "P" */: { @@ -3061,20 +3131,19 @@ export function processBinaryChunk( const resolvedRowTag = chunk[i]; if ( resolvedRowTag === 84 /* "T" */ || - (enableBinaryFlight && - (resolvedRowTag === 65 /* "A" */ || - resolvedRowTag === 79 /* "O" */ || - resolvedRowTag === 111 /* "o" */ || - resolvedRowTag === 85 /* "U" */ || - resolvedRowTag === 83 /* "S" */ || - resolvedRowTag === 115 /* "s" */ || - resolvedRowTag === 76 /* "L" */ || - resolvedRowTag === 108 /* "l" */ || - resolvedRowTag === 71 /* "G" */ || - resolvedRowTag === 103 /* "g" */ || - resolvedRowTag === 77 /* "M" */ || - resolvedRowTag === 109 /* "m" */ || - resolvedRowTag === 86)) /* "V" */ + resolvedRowTag === 65 /* "A" */ || + resolvedRowTag === 79 /* "O" */ || + resolvedRowTag === 111 /* "o" */ || + resolvedRowTag === 85 /* "U" */ || + resolvedRowTag === 83 /* "S" */ || + resolvedRowTag === 115 /* "s" */ || + resolvedRowTag === 76 /* "L" */ || + resolvedRowTag === 108 /* "l" */ || + resolvedRowTag === 71 /* "G" */ || + resolvedRowTag === 103 /* "g" */ || + resolvedRowTag === 77 /* "M" */ || + resolvedRowTag === 109 /* "m" */ || + resolvedRowTag === 86 /* "V" */ ) { rowTag = resolvedRowTag; rowState = ROW_LENGTH; @@ -3187,20 +3256,19 @@ export function processStringChunk(response: Response, chunk: string): void { const resolvedRowTag = chunk.charCodeAt(i); if ( resolvedRowTag === 84 /* "T" */ || - (enableBinaryFlight && - (resolvedRowTag === 65 /* "A" */ || - resolvedRowTag === 79 /* "O" */ || - resolvedRowTag === 111 /* "o" */ || - resolvedRowTag === 85 /* "U" */ || - resolvedRowTag === 83 /* "S" */ || - resolvedRowTag === 115 /* "s" */ || - resolvedRowTag === 76 /* "L" */ || - resolvedRowTag === 108 /* "l" */ || - resolvedRowTag === 71 /* "G" */ || - resolvedRowTag === 103 /* "g" */ || - resolvedRowTag === 77 /* "M" */ || - resolvedRowTag === 109 /* "m" */ || - resolvedRowTag === 86)) /* "V" */ + resolvedRowTag === 65 /* "A" */ || + resolvedRowTag === 79 /* "O" */ || + resolvedRowTag === 111 /* "o" */ || + resolvedRowTag === 85 /* "U" */ || + resolvedRowTag === 83 /* "S" */ || + resolvedRowTag === 115 /* "s" */ || + resolvedRowTag === 76 /* "L" */ || + resolvedRowTag === 108 /* "l" */ || + resolvedRowTag === 71 /* "G" */ || + resolvedRowTag === 103 /* "g" */ || + resolvedRowTag === 77 /* "M" */ || + resolvedRowTag === 109 /* "m" */ || + resolvedRowTag === 86 /* "V" */ ) { rowTag = resolvedRowTag; rowState = ROW_LENGTH; diff --git a/packages/react-client/src/ReactFlightPerformanceTrack.js b/packages/react-client/src/ReactFlightPerformanceTrack.js index f1e7b30280722..d2860e407cc65 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -19,10 +19,31 @@ const supportsUserTiming = const COMPONENTS_TRACK = 'Server Components ⚛'; +const componentsTrackMarker = { + startTime: 0.001, + detail: { + devtools: { + color: 'primary-light', + track: 'Primary', + trackGroup: COMPONENTS_TRACK, + }, + }, +}; + +export function markAllTracksInOrder() { + if (supportsUserTiming) { + // Ensure we create the Server Component track groups earlier than the Client Scheduler + // and Client Components. We can always add the 0 time slot even if it's in the past. + // That's still considered for ordering. + performance.mark('Server Components Track', componentsTrackMarker); + } +} + // Reused to avoid thrashing the GC. const reusableComponentDevToolDetails = { color: 'primary', - track: COMPONENTS_TRACK, + track: '', + trackGroup: COMPONENTS_TRACK, }; const reusableComponentOptions = { start: -0, @@ -32,25 +53,68 @@ const reusableComponentOptions = { }, }; +const trackNames = [ + 'Primary', + 'Parallel', + 'Parallel\u200b', // Padded with zero-width space to give each track a unique name. + 'Parallel\u200b\u200b', + 'Parallel\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b', +]; + export function logComponentRender( componentInfo: ReactComponentInfo, + trackIdx: number, startTime: number, endTime: number, childrenEndTime: number, + rootEnv: string, ): void { - if (supportsUserTiming && childrenEndTime >= 0) { + if (supportsUserTiming && childrenEndTime >= 0 && trackIdx < 10) { + const env = componentInfo.env; const name = componentInfo.name; + const isPrimaryEnv = env === rootEnv; const selfTime = endTime - startTime; reusableComponentDevToolDetails.color = selfTime < 0.5 - ? 'primary-light' + ? isPrimaryEnv + ? 'primary-light' + : 'secondary-light' : selfTime < 50 - ? 'primary' + ? isPrimaryEnv + ? 'primary' + : 'secondary' : selfTime < 500 - ? 'primary-dark' + ? isPrimaryEnv + ? 'primary-dark' + : 'secondary-dark' : 'error'; + reusableComponentDevToolDetails.track = trackNames[trackIdx]; reusableComponentOptions.start = startTime < 0 ? 0 : startTime; reusableComponentOptions.end = childrenEndTime; - performance.measure(name, reusableComponentOptions); + const entryName = + isPrimaryEnv || env === undefined ? name : name + ' [' + env + ']'; + performance.measure(entryName, reusableComponentOptions); + } +} + +export function logDedupedComponentRender( + componentInfo: ReactComponentInfo, + trackIdx: number, + startTime: number, + endTime: number, +): void { + if (supportsUserTiming && endTime >= 0 && trackIdx < 10) { + const name = componentInfo.name; + reusableComponentDevToolDetails.color = 'tertiary-light'; + reusableComponentDevToolDetails.track = trackNames[trackIdx]; + reusableComponentOptions.start = startTime < 0 ? 0 : startTime; + reusableComponentOptions.end = endTime; + const entryName = name + ' [deduped]'; + performance.measure(entryName, reusableComponentOptions); } } diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index bfb2573ea35b0..65d1129b53830 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -18,11 +18,7 @@ import type { import type {LazyComponent} from 'react/src/ReactLazy'; import type {TemporaryReferenceSet} from './ReactFlightTemporaryReferences'; -import { - enableRenderableContext, - enableBinaryFlight, - enableFlightReadableStream, -} from 'shared/ReactFeatureFlags'; +import {enableRenderableContext} from 'shared/ReactFeatureFlags'; import { REACT_ELEMENT_TYPE, @@ -578,73 +574,71 @@ export function processReply( return serializeSetID(setId); } - if (enableBinaryFlight) { - if (value instanceof ArrayBuffer) { - const blob = new Blob([value]); - const blobId = nextPartId++; - if (formData === null) { - formData = new FormData(); - } - formData.append(formFieldPrefix + blobId, blob); - return '$' + 'A' + blobId.toString(16); - } - if (value instanceof Int8Array) { - // char - return serializeTypedArray('O', value); - } - if (value instanceof Uint8Array) { - // unsigned char - return serializeTypedArray('o', value); - } - if (value instanceof Uint8ClampedArray) { - // unsigned clamped char - return serializeTypedArray('U', value); - } - if (value instanceof Int16Array) { - // sort - return serializeTypedArray('S', value); - } - if (value instanceof Uint16Array) { - // unsigned short - return serializeTypedArray('s', value); - } - if (value instanceof Int32Array) { - // long - return serializeTypedArray('L', value); - } - if (value instanceof Uint32Array) { - // unsigned long - return serializeTypedArray('l', value); - } - if (value instanceof Float32Array) { - // float - return serializeTypedArray('G', value); - } - if (value instanceof Float64Array) { - // double - return serializeTypedArray('g', value); - } - if (value instanceof BigInt64Array) { - // number - return serializeTypedArray('M', value); - } - if (value instanceof BigUint64Array) { - // unsigned number - // We use "m" instead of "n" since JSON can start with "null" - return serializeTypedArray('m', value); - } - if (value instanceof DataView) { - return serializeTypedArray('V', value); + if (value instanceof ArrayBuffer) { + const blob = new Blob([value]); + const blobId = nextPartId++; + if (formData === null) { + formData = new FormData(); } - // TODO: Blob is not available in old Node/browsers. Remove the typeof check later. - if (typeof Blob === 'function' && value instanceof Blob) { - if (formData === null) { - formData = new FormData(); - } - const blobId = nextPartId++; - formData.append(formFieldPrefix + blobId, value); - return serializeBlobID(blobId); + formData.append(formFieldPrefix + blobId, blob); + return '$' + 'A' + blobId.toString(16); + } + if (value instanceof Int8Array) { + // char + return serializeTypedArray('O', value); + } + if (value instanceof Uint8Array) { + // unsigned char + return serializeTypedArray('o', value); + } + if (value instanceof Uint8ClampedArray) { + // unsigned clamped char + return serializeTypedArray('U', value); + } + if (value instanceof Int16Array) { + // sort + return serializeTypedArray('S', value); + } + if (value instanceof Uint16Array) { + // unsigned short + return serializeTypedArray('s', value); + } + if (value instanceof Int32Array) { + // long + return serializeTypedArray('L', value); + } + if (value instanceof Uint32Array) { + // unsigned long + return serializeTypedArray('l', value); + } + if (value instanceof Float32Array) { + // float + return serializeTypedArray('G', value); + } + if (value instanceof Float64Array) { + // double + return serializeTypedArray('g', value); + } + if (value instanceof BigInt64Array) { + // number + return serializeTypedArray('M', value); + } + if (value instanceof BigUint64Array) { + // unsigned number + // We use "m" instead of "n" since JSON can start with "null" + return serializeTypedArray('m', value); + } + if (value instanceof DataView) { + return serializeTypedArray('V', value); + } + // TODO: Blob is not available in old Node/browsers. Remove the typeof check later. + if (typeof Blob === 'function' && value instanceof Blob) { + if (formData === null) { + formData = new FormData(); } + const blobId = nextPartId++; + formData.append(formFieldPrefix + blobId, value); + return serializeBlobID(blobId); } const iteratorFn = getIteratorFn(value); @@ -666,23 +660,21 @@ export function processReply( return Array.from((iterator: any)); } - if (enableFlightReadableStream) { - // TODO: ReadableStream is not available in old Node. Remove the typeof check later. - if ( - typeof ReadableStream === 'function' && - value instanceof ReadableStream - ) { - return serializeReadableStream(value); - } - const getAsyncIterator: void | (() => $AsyncIterator) = - (value: any)[ASYNC_ITERATOR]; - if (typeof getAsyncIterator === 'function') { - // We treat AsyncIterables as a Fragment and as such we might need to key them. - return serializeAsyncIterable( - (value: any), - getAsyncIterator.call((value: any)), - ); - } + // TODO: ReadableStream is not available in old Node. Remove the typeof check later. + if ( + typeof ReadableStream === 'function' && + value instanceof ReadableStream + ) { + return serializeReadableStream(value); + } + const getAsyncIterator: void | (() => $AsyncIterator) = + (value: any)[ASYNC_ITERATOR]; + if (typeof getAsyncIterator === 'function') { + // We treat AsyncIterables as a Fragment and as such we might need to key them. + return serializeAsyncIterable( + (value: any), + getAsyncIterator.call((value: any)), + ); } // Verify that this is a simple plain object. diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 75220b762918e..b2489705394a1 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -2059,7 +2059,7 @@ describe('ReactFlight', () => { expect(errors).toEqual(['Cannot pass a secret token to the client']); }); - // @gate enableTaint && enableBinaryFlight + // @gate enableTaint it('errors when a tainted binary value is serialized', async () => { function UserClient({user}) { return {user.name}; @@ -2623,7 +2623,7 @@ describe('ReactFlight', () => { ); }); - // @gate enableFlightReadableStream && enableAsyncIterableChildren + // @gate enableAsyncIterableChildren it('shares state when moving keyed Server Components that render async iterables', async () => { function StatefulClient({name, initial}) { const [state] = React.useState(initial); @@ -2814,7 +2814,7 @@ describe('ReactFlight', () => { ); }); - // @gate enableFlightReadableStream && enableAsyncIterableChildren + // @gate enableAsyncIterableChildren it('preserves debug info for server-to-server pass through of async iterables', async () => { let resolve; const iteratorPromise = new Promise(r => (resolve = r)); @@ -2849,10 +2849,9 @@ describe('ReactFlight', () => { }, ); - if (gate(flag => flag.enableFlightReadableStream)) { - // Wait for the iterator to finish - await iteratorPromise; - } + // Wait for the iterator to finish + await iteratorPromise; + await 0; // One more tick for the return value / closing. const transport = ReactNoopFlightServer.render( diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index eccaa6a67b166..d2cea81f576c4 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -20,7 +20,6 @@ import type { Dependencies, Fiber, Dispatcher as DispatcherType, - ContextDependencyWithSelect, } from 'react-reconciler/src/ReactInternalTypes'; import type {TransitionStatus} from 'react-reconciler/src/ReactFiberConfig'; @@ -76,13 +75,6 @@ function getPrimitiveStackCache(): Map> { try { // Use all hooks here to add them to the hook log. Dispatcher.useContext(({_currentValue: null}: any)); - if (typeof Dispatcher.unstable_useContextWithBailout === 'function') { - // This type check is for Flow only. - Dispatcher.unstable_useContextWithBailout( - ({_currentValue: null}: any), - null, - ); - } Dispatcher.useState(null); Dispatcher.useReducer((s: mixed, a: mixed) => s, null); Dispatcher.useRef(null); @@ -150,10 +142,7 @@ function getPrimitiveStackCache(): Map> { let currentFiber: null | Fiber = null; let currentHook: null | Hook = null; -let currentContextDependency: - | null - | ContextDependency - | ContextDependencyWithSelect = null; +let currentContextDependency: null | ContextDependency = null; function nextHook(): null | Hook { const hook = currentHook; @@ -274,22 +263,6 @@ function useContext(context: ReactContext): T { return value; } -function unstable_useContextWithBailout( - context: ReactContext, - select: (T => Array) | null, -): T { - const value = readContext(context); - hookLog.push({ - displayName: context.displayName || null, - primitive: 'ContextWithBailout', - stackError: new Error(), - value: value, - debugInfo: null, - dispatcherHookName: 'ContextWithBailout', - }); - return value; -} - function useState( initialState: (() => S) | S, ): [S, Dispatch>] { @@ -764,7 +737,6 @@ const Dispatcher: DispatcherType = { useCacheRefresh, useCallback, useContext, - unstable_useContextWithBailout, useEffect, useImperativeHandle, useDebugValue, diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js index fcd84fedfed71..ff8e7e1ac8683 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js @@ -1573,7 +1573,6 @@ describe('ReactHooksInspectionIntegration', () => { }); describe('useMemoCache', () => { - // @gate enableUseMemoCacheHook it('should not be inspectable', async () => { function Foo() { const $ = useMemoCache(1); @@ -1601,7 +1600,6 @@ describe('ReactHooksInspectionIntegration', () => { expect(tree.length).toEqual(0); }); - // @gate enableUseMemoCacheHook it('should work in combination with other hooks', async () => { function useSomething() { const [something] = React.useState(null); diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index ac95c91c596a8..05892c930e1ca 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -63,10 +63,7 @@ import {validateProperties as validateInputProperties} from '../shared/ReactDOMN import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook'; import sanitizeURL from '../shared/sanitizeURL'; -import { - enableTrustedTypesIntegration, - enableFilterEmptyStringAttributesDOM, -} from 'shared/ReactFeatureFlags'; +import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags'; import { mediaEventTypes, listenToNonDelegatedEvent, @@ -400,35 +397,33 @@ function setProp( // fallthrough case 'src': case 'href': { - if (enableFilterEmptyStringAttributesDOM) { - if ( - value === '' && - // is fine for "reload" links. - !(tag === 'a' && key === 'href') - ) { - if (__DEV__) { - if (key === 'src') { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'This may cause the browser to download the whole page again over the network. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - key, - key, - ); - } else { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - key, - key, - ); - } + if ( + value === '' && + // is fine for "reload" links. + !(tag === 'a' && key === 'href') + ) { + if (__DEV__) { + if (key === 'src') { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'This may cause the browser to download the whole page again over the network. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + key, + key, + ); + } else { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + key, + key, + ); } - domElement.removeAttribute(key); - break; } + domElement.removeAttribute(key); + break; } if ( value == null || @@ -2489,53 +2484,52 @@ function diffHydratedGenericElement( // fallthrough case 'src': case 'href': - if (enableFilterEmptyStringAttributesDOM) { - if ( - value === '' && - // is fine for "reload" links. - !(tag === 'a' && propKey === 'href') && - !(tag === 'object' && propKey === 'data') - ) { - if (__DEV__) { - if (propKey === 'src') { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'This may cause the browser to download the whole page again over the network. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - propKey, - propKey, - ); - } else { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - propKey, - propKey, - ); - } + if ( + value === '' && + // is fine for "reload" links. + !(tag === 'a' && propKey === 'href') && + !(tag === 'object' && propKey === 'data') + ) { + if (__DEV__) { + if (propKey === 'src') { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'This may cause the browser to download the whole page again over the network. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + propKey, + propKey, + ); + } else { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + propKey, + propKey, + ); } - hydrateSanitizedAttribute( - domElement, - propKey, - propKey, - null, - extraAttributes, - serverDifferences, - ); - continue; } + hydrateSanitizedAttribute( + domElement, + propKey, + propKey, + null, + extraAttributes, + serverDifferences, + ); + continue; + } else { + hydrateSanitizedAttribute( + domElement, + propKey, + propKey, + value, + extraAttributes, + serverDifferences, + ); + continue; } - hydrateSanitizedAttribute( - domElement, - propKey, - propKey, - value, - extraAttributes, - serverDifferences, - ); - continue; case 'action': case 'formAction': { const serverValue = domElement.getAttribute(propKey); diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 1351a28db3aab..04aaf0c2ac706 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -27,10 +27,7 @@ import { import {Children} from 'react'; -import { - enableFilterEmptyStringAttributesDOM, - enableFizzExternalRuntime, -} from 'shared/ReactFeatureFlags'; +import {enableFizzExternalRuntime} from 'shared/ReactFeatureFlags'; import type { Destination, @@ -1210,30 +1207,28 @@ function pushAttribute( } case 'src': case 'href': { - if (enableFilterEmptyStringAttributesDOM) { - if (value === '') { - if (__DEV__) { - if (name === 'src') { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'This may cause the browser to download the whole page again over the network. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - name, - name, - ); - } else { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - name, - name, - ); - } + if (value === '') { + if (__DEV__) { + if (name === 'src') { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'This may cause the browser to download the whole page again over the network. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + name, + name, + ); + } else { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + name, + name, + ); } - return; } + return; } } // Fall through to the last case which shouldn't remove empty strings. @@ -1633,19 +1628,17 @@ function pushStartObject( checkAttributeStringCoercion(propValue, 'data'); } const sanitizedValue = sanitizeURL('' + propValue); - if (enableFilterEmptyStringAttributesDOM) { - if (sanitizedValue === '') { - if (__DEV__) { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - propKey, - propKey, - ); - } - break; + if (sanitizedValue === '') { + if (__DEV__) { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + propKey, + propKey, + ); } + break; } target.push( attributeSeparator, @@ -3087,7 +3080,7 @@ function pushTitle( console.error( 'React expects the `children` prop of tags to be a string, number, bigint, or object with a novel `toString` method but found an Array with length %s instead.' + ' Browsers treat all child Nodes of <title> tags as Text content and React expects to be able to convert `children` of <title> tags to a single string value' + - ' which is why Arrays of length greater than 1 are not supported. When using JSX it can be commong to combine text nodes and value nodes.' + + ' which is why Arrays of length greater than 1 are not supported. When using JSX it can be common to combine text nodes and value nodes.' + ' For example: <title>hello {nameOfUser}. While not immediately apparent, `children` in this case is an Array with length 2. If your `children` prop' + ' is using this form try rewriting it using a template string: {`hello ${nameOfUser}`}.', children.length, @@ -3615,11 +3608,7 @@ export function pushStartInstance( // Fast track very common tags break; case 'a': - if (enableFilterEmptyStringAttributesDOM) { - return pushStartAnchor(target, props); - } else { - break; - } + return pushStartAnchor(target, props); case 'g': case 'p': case 'li': @@ -3897,18 +3886,6 @@ const clientRenderedSuspenseBoundaryError1D = const clientRenderedSuspenseBoundaryError2 = stringToPrecomputedChunk('>'); -export function pushStartCompletedSuspenseBoundary( - target: Array, -) { - target.push(startCompletedSuspenseBoundary); -} - -export function pushEndCompletedSuspenseBoundary( - target: Array, -) { - target.push(endSuspenseBoundary); -} - export function writeStartCompletedSuspenseBoundary( destination: Destination, renderState: RenderState, diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js index 725ee666f52af..e4cd9ca93d99e 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js @@ -142,8 +142,6 @@ export { makeId, pushStartInstance, pushEndInstance, - pushStartCompletedSuspenseBoundary, - pushEndCompletedSuspenseBoundary, pushFormStateMarkerIsMatching, pushFormStateMarkerIsNotMatching, writeStartSegment, diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 4444076681208..30b5ced5cf2e3 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -537,7 +537,6 @@ describe('ReactCompositeComponent', () => { }); it('should cleanup even if render() fatals', async () => { - const dispatcherEnabled = __DEV__ || gate(flags => flags.enableCache); const ownerEnabled = __DEV__; let stashedDispatcher; @@ -551,7 +550,7 @@ describe('ReactCompositeComponent', () => { } const instance = ; - expect(ReactSharedInternals.A).toBe(dispatcherEnabled ? null : undefined); + expect(ReactSharedInternals.A).toBe(null); const root = ReactDOMClient.createRoot(document.createElement('div')); await expect(async () => { @@ -560,15 +559,11 @@ describe('ReactCompositeComponent', () => { }); }).rejects.toThrow(); - expect(ReactSharedInternals.A).toBe(dispatcherEnabled ? null : undefined); - if (dispatcherEnabled) { - if (ownerEnabled) { - expect(stashedDispatcher.getOwner()).toBe(null); - } else { - expect(stashedDispatcher.getOwner).toBe(undefined); - } + expect(ReactSharedInternals.A).toBe(null); + if (ownerEnabled) { + expect(stashedDispatcher.getOwner()).toBe(null); } else { - expect(stashedDispatcher).toBe(undefined); + expect(stashedDispatcher.getOwner).toBe(undefined); } }); diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index d37a4ecba6dc6..ce71a6334ee64 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -587,133 +587,131 @@ describe('ReactDOMComponent', () => { expect(node.hasAttribute('data-foo')).toBe(false); }); - if (ReactFeatureFlags.enableFilterEmptyStringAttributesDOM) { - it('should not add an empty src attribute', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'An empty string ("") was passed to the src attribute. ' + - 'This may cause the browser to download the whole page again over the network. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to src instead of an empty string.', - ); - const node = container.firstChild; - expect(node.hasAttribute('src')).toBe(false); - + it('should not add an empty src attribute', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect(async () => { await act(() => { - root.render(); + root.render(); }); - expect(node.hasAttribute('src')).toBe(true); + }).toErrorDev( + 'An empty string ("") was passed to the src attribute. ' + + 'This may cause the browser to download the whole page again over the network. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to src instead of an empty string.', + ); + const node = container.firstChild; + expect(node.hasAttribute('src')).toBe(false); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'An empty string ("") was passed to the src attribute. ' + - 'This may cause the browser to download the whole page again over the network. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to src instead of an empty string.', - ); - expect(node.hasAttribute('src')).toBe(false); + await act(() => { + root.render(); }); + expect(node.hasAttribute('src')).toBe(true); - it('should not add an empty href attribute', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'An empty string ("") was passed to the href attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to href instead of an empty string.', - ); - const node = container.firstChild; - expect(node.hasAttribute('href')).toBe(false); + await expect(async () => { + await act(() => { + root.render(); + }); + }).toErrorDev( + 'An empty string ("") was passed to the src attribute. ' + + 'This may cause the browser to download the whole page again over the network. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to src instead of an empty string.', + ); + expect(node.hasAttribute('src')).toBe(false); + }); + it('should not add an empty href attribute', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect(async () => { await act(() => { - root.render(); + root.render(); }); - expect(node.hasAttribute('href')).toBe(true); + }).toErrorDev( + 'An empty string ("") was passed to the href attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to href instead of an empty string.', + ); + const node = container.firstChild; + expect(node.hasAttribute('href')).toBe(false); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'An empty string ("") was passed to the href attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to href instead of an empty string.', - ); - expect(node.hasAttribute('href')).toBe(false); + await act(() => { + root.render(); }); + expect(node.hasAttribute('href')).toBe(true); - it('should allow an empty href attribute on anchors', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); + await expect(async () => { await act(() => { - root.render(); + root.render(); }); - const node = container.firstChild; - expect(node.getAttribute('href')).toBe(''); + }).toErrorDev( + 'An empty string ("") was passed to the href attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to href instead of an empty string.', + ); + expect(node.hasAttribute('href')).toBe(false); + }); + + it('should allow an empty href attribute on anchors', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); }); + const node = container.firstChild; + expect(node.getAttribute('href')).toBe(''); + }); - it('should allow an empty action attribute', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(
); - }); - const node = container.firstChild; - expect(node.getAttribute('action')).toBe(''); + it('should allow an empty action attribute', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + const node = container.firstChild; + expect(node.getAttribute('action')).toBe(''); - await act(() => { - root.render(); - }); - expect(node.hasAttribute('action')).toBe(true); + await act(() => { + root.render(); + }); + expect(node.hasAttribute('action')).toBe(true); - await act(() => { - root.render(); - }); - expect(node.getAttribute('action')).toBe(''); + await act(() => { + root.render(); }); + expect(node.getAttribute('action')).toBe(''); + }); - it('allows empty string of a formAction to override the default of a parent', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - -