From 5f232d72d4cb15470c77e91c30e9cf61e090508c Mon Sep 17 00:00:00 2001 From: lauren Date: Wed, 26 Mar 2025 13:13:39 -0400 Subject: [PATCH 01/13] [ci] Skip yarn install on cache hit (#32757) We currently already do this in runtime_build_and_test, we can reuse the same technique in other workflows to speed them up. --- .github/workflows/compiler_playground.yml | 6 ++++- .github/workflows/compiler_prereleases.yml | 3 ++- .github/workflows/compiler_typescript.yml | 10 ++++++--- .../workflows/devtools_regression_tests.yml | 22 +++++++++++++++---- .../workflows/runtime_eslint_plugin_e2e.yml | 11 ++++++---- .github/workflows/runtime_prereleases.yml | 2 ++ .../runtime_releases_from_npm_manual.yml | 2 ++ .github/workflows/shared_lint.yml | 8 +++++++ 8 files changed, 51 insertions(+), 13 deletions(-) diff --git a/.github/workflows/compiler_playground.yml b/.github/workflows/compiler_playground.yml index edd95e365e..34349f584e 100644 --- a/.github/workflows/compiler_playground.yml +++ b/.github/workflows/compiler_playground.yml @@ -40,8 +40,12 @@ jobs: with: path: | **/node_modules - key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/**/yarn.lock') }} + key: compiler-and-playground-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/**/yarn.lock') }} - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' + working-directory: compiler + - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - name: Check Playwright version id: playwright_version run: echo "playwright_version=$(npm ls @playwright/test | grep @playwright | sed 's/.*@//' | head -1)" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/compiler_prereleases.yml b/.github/workflows/compiler_prereleases.yml index 7928bd430e..2bb2c6ef16 100644 --- a/.github/workflows/compiler_prereleases.yml +++ b/.github/workflows/compiler_prereleases.yml @@ -49,8 +49,9 @@ jobs: with: path: | **/node_modules - key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/**/yarn.lock') }} + key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/yarn.lock') }} - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - name: Publish packages to npm run: | cp ./scripts/release/ci-npmrc ~/.npmrc diff --git a/.github/workflows/compiler_typescript.yml b/.github/workflows/compiler_typescript.yml index 9c749a3bfb..6a3b52e21a 100644 --- a/.github/workflows/compiler_typescript.yml +++ b/.github/workflows/compiler_typescript.yml @@ -47,11 +47,13 @@ jobs: cache-dependency-path: compiler/yarn.lock - name: Restore cached node_modules uses: actions/cache@v4 + id: node_modules with: path: | **/node_modules - key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/**/yarn.lock') }} + key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/yarn.lock') }} - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn workspace babel-plugin-react-compiler lint # Hardcoded to improve parallelism @@ -71,8 +73,9 @@ jobs: with: path: | **/node_modules - key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/**/yarn.lock') }} + key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/yarn.lock') }} - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn workspace babel-plugin-react-compiler jest test: @@ -96,8 +99,9 @@ jobs: with: path: | **/node_modules - key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/**/yarn.lock') }} + key: compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('compiler/yarn.lock') }} - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: xvfb-run -a yarn workspace ${{ matrix.workspace_name }} test if: runner.os == 'Linux' && matrix.workspace_name == 'react-forgive' - run: yarn workspace ${{ matrix.workspace_name }} test diff --git a/.github/workflows/devtools_regression_tests.yml b/.github/workflows/devtools_regression_tests.yml index cb6a5b68a8..0b70cfaf4e 100644 --- a/.github/workflows/devtools_regression_tests.yml +++ b/.github/workflows/devtools_regression_tests.yml @@ -40,7 +40,9 @@ jobs: - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn --cwd scripts/release install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - name: Download react-devtools artifacts for base revision run: | git fetch origin main @@ -75,6 +77,7 @@ jobs: - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - name: Restore archived build uses: actions/download-artifact@v4 with: @@ -134,6 +137,7 @@ jobs: **/node_modules key: runtime-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('yarn.lock') }} - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - name: Restore all archived build artifacts uses: actions/download-artifact@v4 - name: Display structure of build @@ -169,14 +173,24 @@ jobs: **/node_modules key: runtime-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('yarn.lock') }} - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - name: Restore all archived build artifacts uses: actions/download-artifact@v4 - name: Display structure of build run: ls -R build - - name: Playwright install deps - run: | - npx playwright install - sudo npx playwright install-deps + - name: Check Playwright version + id: playwright_version + run: echo "playwright_version=$(npm ls @playwright/test | grep @playwright | sed 's/.*@//' | head -1)" >> "$GITHUB_OUTPUT" + - name: Cache Playwright Browsers for version ${{ steps.playwright_version.outputs.playwright_version }} + id: cache_playwright_browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: playwright-browsers-v6-${{ runner.arch }}-${{ runner.os }}-${{ steps.playwright_version.outputs.playwright_version }} + - run: npx playwright install --with-deps + if: steps.cache_playwright_browsers.outputs.cache-hit != 'true' + - run: npx playwright install-deps + if: steps.cache_playwright_browsers.outputs.cache-hit == 'true' - run: ./scripts/ci/download_devtools_regression_build.js ${{ matrix.version }} - run: ls -R build-regression - run: ./scripts/ci/run_devtools_e2e_tests.js ${{ matrix.version }} diff --git a/.github/workflows/runtime_eslint_plugin_e2e.yml b/.github/workflows/runtime_eslint_plugin_e2e.yml index 9b3d134204..92921646c1 100644 --- a/.github/workflows/runtime_eslint_plugin_e2e.yml +++ b/.github/workflows/runtime_eslint_plugin_e2e.yml @@ -46,17 +46,20 @@ jobs: with: path: | **/node_modules - key: runtime-and-compiler-eslint_e2e-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('yarn.lock', 'compiler/yarn.lock') }} + key: runtime-and-compiler-eslint_e2e-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('yarn.lock', 'compiler/yarn.lock', 'fixtures/eslint-v*/yarn.lock') }} - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn --cwd compiler install --frozen-lockfile - - name: Build plugin - working-directory: fixtures/eslint-v${{ matrix.eslint_major }} - run: node build.mjs + if: steps.node_modules.outputs.cache-hit != 'true' - name: Install fixture dependencies working-directory: ./fixtures/eslint-v${{ matrix.eslint_major }} run: yarn --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' + - name: Build plugin + working-directory: fixtures/eslint-v${{ matrix.eslint_major }} + run: node build.mjs - name: Run lint test working-directory: ./fixtures/eslint-v${{ matrix.eslint_major }} run: yarn lint diff --git a/.github/workflows/runtime_prereleases.yml b/.github/workflows/runtime_prereleases.yml index 1449f6af5a..e3cd5bd1a2 100644 --- a/.github/workflows/runtime_prereleases.yml +++ b/.github/workflows/runtime_prereleases.yml @@ -51,7 +51,9 @@ jobs: - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn --cwd scripts/release install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: | GH_TOKEN=${{ secrets.GH_TOKEN }} scripts/release/prepare-release-from-ci.js --skipTests -r ${{ inputs.release_channel }} --commit=${{ inputs.commit_sha }} cp ./scripts/release/ci-npmrc ~/.npmrc diff --git a/.github/workflows/runtime_releases_from_npm_manual.yml b/.github/workflows/runtime_releases_from_npm_manual.yml index 4bc3957486..51e3843955 100644 --- a/.github/workflows/runtime_releases_from_npm_manual.yml +++ b/.github/workflows/runtime_releases_from_npm_manual.yml @@ -78,7 +78,9 @@ jobs: - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn --cwd scripts/release install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: cp ./scripts/release/ci-npmrc ~/.npmrc - if: '${{ inputs.only_packages }}' name: 'Prepare ${{ inputs.only_packages }} from NPM' diff --git a/.github/workflows/shared_lint.yml b/.github/workflows/shared_lint.yml index e14e9a252b..3c359cff22 100644 --- a/.github/workflows/shared_lint.yml +++ b/.github/workflows/shared_lint.yml @@ -29,6 +29,7 @@ jobs: cache-dependency-path: yarn.lock - name: Restore cached node_modules uses: actions/cache@v4 + id: node_modules with: path: | **/node_modules @@ -36,6 +37,7 @@ jobs: - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn prettier-check eslint: @@ -50,6 +52,7 @@ jobs: cache-dependency-path: yarn.lock - name: Restore cached node_modules uses: actions/cache@v4 + id: node_modules with: path: | **/node_modules @@ -57,6 +60,7 @@ jobs: - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: node ./scripts/tasks/eslint check_license: @@ -71,6 +75,7 @@ jobs: cache-dependency-path: yarn.lock - name: Restore cached node_modules uses: actions/cache@v4 + id: node_modules with: path: | **/node_modules @@ -78,6 +83,7 @@ jobs: - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: ./scripts/ci/check_license.sh test_print_warnings: @@ -92,6 +98,7 @@ jobs: cache-dependency-path: yarn.lock - name: Restore cached node_modules uses: actions/cache@v4 + id: node_modules with: path: | **/node_modules @@ -99,4 +106,5 @@ jobs: - name: Ensure clean build directory run: rm -rf build - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' - run: ./scripts/ci/test_print_warnings.sh From 33999c43177e13580730c2fad94a77f4b0e08ef2 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Wed, 26 Mar 2025 14:27:42 -0400 Subject: [PATCH 02/13] [compiler][be] Test runner (snap) now uses tsup bundled plugin (#32758) Currently, `babel-plugin-react-compiler` is bundled with (almost) all external dependencies. This is because babel traversal and ast logic is not forward-compatible. Since `babel-plugin-react-compiler` needs to be compatible with babel pipelines across a wide semvar range, we (1) set this package's babel dependency to an early version and (2) inline babel libraries into our bundle. A few other packages in `react/compiler` depend on the compiler. This PR moves `snap`, our test fixture compiler and evaluator, to use the bundled version of `babel-plugin-react-compiler`. This decouples the babel version used by `snap` with the version used by `babel-plugin-react-compiler`, which means that `snap` now can test features from newer babel versions (see https://github.com/facebook/react/pull/32742). --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32758). * #32759 * __->__ #32758 --- .../babel-plugin-react-compiler/package.json | 2 +- .../src/HIR/index.ts | 2 +- .../src/ReactiveScopes/index.ts | 5 +- .../babel-plugin-react-compiler/src/index.ts | 6 ++- .../babel-plugin-react-compiler/tsconfig.json | 2 +- compiler/packages/snap/package.json | 4 +- compiler/packages/snap/src/constants.ts | 36 ++++--------- compiler/packages/snap/src/runner-watch.ts | 32 +++++++---- compiler/packages/snap/src/runner-worker.ts | 53 +++++++++++-------- compiler/packages/snap/src/runner.ts | 37 +++++++------ compiler/yarn.lock | 11 +++- 11 files changed, 105 insertions(+), 85 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/package.json b/compiler/packages/babel-plugin-react-compiler/package.json index 7544903f8f..f3ded80a49 100644 --- a/compiler/packages/babel-plugin-react-compiler/package.json +++ b/compiler/packages/babel-plugin-react-compiler/package.json @@ -12,7 +12,7 @@ "build": "rimraf dist && tsup", "test": "./scripts/link-react-compiler-runtime.sh && yarn snap:ci", "jest": "yarn build && ts-node node_modules/.bin/jest", - "snap": "node ../snap/dist/main.js", + "snap": "yarn workspace snap run snap", "snap:build": "yarn workspace snap run build", "snap:ci": "yarn snap:build && yarn snap", "ts:analyze-trace": "scripts/ts-analyze-trace.sh", diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/index.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/index.ts index 45267dd9a1..579c525dfb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/index.ts @@ -32,5 +32,5 @@ export { } from './HIRBuilder'; export {mergeConsecutiveBlocks} from './MergeConsecutiveBlocks'; export {mergeOverlappingReactiveScopesHIR} from './MergeOverlappingReactiveScopesHIR'; -export {printFunction, printHIR} from './PrintHIR'; +export {printFunction, printHIR, printFunctionWithOutlined} from './PrintHIR'; export {pruneUnusedLabelsHIR} from './PruneUnusedLabelsHIR'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/index.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/index.ts index 8841ae9279..d0f89f05d2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/index.ts @@ -14,7 +14,10 @@ export {extractScopeDeclarationsFromDestructuring} from './ExtractScopeDeclarati export {inferReactiveScopeVariables} from './InferReactiveScopeVariables'; export {memoizeFbtAndMacroOperandsInSameScope} from './MemoizeFbtAndMacroOperandsInSameScope'; export {mergeReactiveScopesThatInvalidateTogether} from './MergeReactiveScopesThatInvalidateTogether'; -export {printReactiveFunction} from './PrintReactiveFunction'; +export { + printReactiveFunction, + printReactiveFunctionWithOutlined, +} from './PrintReactiveFunction'; export {promoteUsedTemporaries} from './PromoteUsedTemporaries'; export {propagateEarlyReturns} from './PropagateEarlyReturns'; export {pruneAllReactiveScopes} from './PruneAllReactiveScopes'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/index.ts b/compiler/packages/babel-plugin-react-compiler/src/index.ts index 3310581462..60865b8aa7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/index.ts @@ -32,13 +32,17 @@ export { ValueKind, parseConfigPragmaForTests, printHIR, + printFunctionWithOutlined, validateEnvironmentConfig, type EnvironmentConfig, type ExternalFunction, type Hook, type SourceLocation, } from './HIR'; -export {printReactiveFunction} from './ReactiveScopes'; +export { + printReactiveFunction, + printReactiveFunctionWithOutlined, +} from './ReactiveScopes'; declare global { let __DEV__: boolean | null | undefined; } diff --git a/compiler/packages/babel-plugin-react-compiler/tsconfig.json b/compiler/packages/babel-plugin-react-compiler/tsconfig.json index 4e4614aef4..afca5f9beb 100644 --- a/compiler/packages/babel-plugin-react-compiler/tsconfig.json +++ b/compiler/packages/babel-plugin-react-compiler/tsconfig.json @@ -4,7 +4,7 @@ "module": "ES2015", "moduleResolution": "Bundler", "rootDir": "src", - "outDir": "dist", + "noEmit": true, "jsx": "react-jsxdev", // weaken strictness from preset "importsNotUsedAsValues": "remove", diff --git a/compiler/packages/snap/package.json b/compiler/packages/snap/package.json index ab6c7846ad..60530f01dd 100644 --- a/compiler/packages/snap/package.json +++ b/compiler/packages/snap/package.json @@ -11,6 +11,7 @@ "scripts": { "postinstall": "./scripts/link-react-compiler-runtime.sh && perl -p -i -e 's/react\\.element/react.transitional.element/' ../../node_modules/fbt/lib/FbtReactUtil.js && perl -p -i -e 's/didWarnAboutUsingAct = false;/didWarnAboutUsingAct = true;/' ../../node_modules/react-dom/cjs/react-dom-test-utils.development.js", "build": "rimraf dist && concurrently -n snap,runtime \"tsc --build\" \"yarn --silent workspace react-compiler-runtime build\"", + "snap": "node dist/main.js", "test": "echo 'no tests'", "prettier": "prettier --write 'src/**/*.ts'" }, @@ -40,8 +41,7 @@ }, "devDependencies": { "@babel/core": "^7.19.1", - "@babel/parser": "^7.19.1", - "@babel/plugin-syntax-typescript": "^7.18.6", + "@babel/parser": "^7.20.15", "@babel/plugin-transform-modules-commonjs": "^7.18.6", "@babel/preset-react": "^7.18.6", "@babel/traverse": "^7.19.1", diff --git a/compiler/packages/snap/src/constants.ts b/compiler/packages/snap/src/constants.ts index ad77441b53..d1ede2a2f2 100644 --- a/compiler/packages/snap/src/constants.ts +++ b/compiler/packages/snap/src/constants.ts @@ -9,35 +9,17 @@ import path from 'path'; // We assume this is run from `babel-plugin-react-compiler` export const PROJECT_ROOT = path.normalize( - path.join(process.cwd(), '..', '..'), + path.join(process.cwd(), '..', 'babel-plugin-react-compiler'), ); -export const COMPILER_PATH = path.join( - process.cwd(), - 'dist', - 'Babel', - 'BabelPlugin.js', -); -export const COMPILER_INDEX_PATH = path.join(process.cwd(), 'dist', 'index'); -export const PRINT_HIR_PATH = path.join( - process.cwd(), - 'dist', - '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(), - 'dist', - 'HIR', - 'Environment.js', + +export const PROJECT_SRC = path.normalize( + path.join(PROJECT_ROOT, 'dist', 'index.js'), ); +export const PRINT_HIR_IMPORT = 'printFunctionWithOutlined'; +export const PRINT_REACTIVE_IR_IMPORT = 'printReactiveFunction'; +export const PARSE_CONFIG_PRAGMA_IMPORT = 'parseConfigPragmaForTests'; export const FIXTURES_PATH = path.join( - process.cwd(), + PROJECT_ROOT, 'src', '__tests__', 'fixtures', @@ -45,4 +27,4 @@ export const FIXTURES_PATH = path.join( ); export const SNAPSHOT_EXTENSION = '.expect.md'; export const FILTER_FILENAME = 'testfilter.txt'; -export const FILTER_PATH = path.join(process.cwd(), FILTER_FILENAME); +export const FILTER_PATH = path.join(PROJECT_ROOT, FILTER_FILENAME); diff --git a/compiler/packages/snap/src/runner-watch.ts b/compiler/packages/snap/src/runner-watch.ts index aa432729ab..e6632d83ed 100644 --- a/compiler/packages/snap/src/runner-watch.ts +++ b/compiler/packages/snap/src/runner-watch.ts @@ -8,15 +8,16 @@ import watcher from '@parcel/watcher'; import path from 'path'; import ts from 'typescript'; -import {FILTER_FILENAME, FIXTURES_PATH} from './constants'; +import {FILTER_FILENAME, FIXTURES_PATH, PROJECT_ROOT} from './constants'; import {TestFilter, readTestFilter} from './fixture-utils'; +import {execSync} from 'child_process'; export function watchSrc( onStart: () => void, onComplete: (isSuccess: boolean) => void, ): ts.WatchOfConfigFile { const configPath = ts.findConfigFile( - /*searchPath*/ './', + /*searchPath*/ PROJECT_ROOT, ts.sys.fileExists, 'tsconfig.json', ); @@ -26,10 +27,7 @@ export function watchSrc( const createProgram = ts.createSemanticDiagnosticsBuilderProgram; const host = ts.createWatchCompilerHost( configPath, - ts.convertCompilerOptionsFromJson( - {module: 'commonjs', outDir: 'dist', sourceMap: true}, - '.', - ).options, + undefined, ts.sys, createProgram, () => {}, // we manually report errors in afterProgramCreate @@ -41,9 +39,11 @@ export function watchSrc( onStart(); return origCreateProgram(rootNames, options, host, oldProgram); }; - const origPostProgramCreate = host.afterProgramCreate; host.afterProgramCreate = program => { - origPostProgramCreate!(program); + /** + * Avoid calling original postProgramCreate because it always emits tsc + * compilation output + */ // syntactic diagnostics refer to javascript syntax const errors = program @@ -172,13 +172,23 @@ function subscribeTsc( // Notify the user when compilation starts but don't clear the screen yet console.log('\nCompiling...'); }, - isSuccess => { + isTypecheckSuccess => { + let isCompilerBuildValid = false; + if (isTypecheckSuccess) { + try { + execSync('yarn build', {cwd: PROJECT_ROOT}); + console.log('Built compiler successfully with tsup'); + isCompilerBuildValid = true; + } catch (e) { + console.warn('Failed to build compiler with tsup:', e); + } + } // Bump the compiler version after a build finishes // and re-run tests - if (isSuccess) { + if (isCompilerBuildValid) { state.compilerVersion++; } - state.isCompilerBuildValid = isSuccess; + state.isCompilerBuildValid = isCompilerBuildValid; state.mode.action = RunnerAction.Test; onChange(state); }, diff --git a/compiler/packages/snap/src/runner-worker.ts b/compiler/packages/snap/src/runner-worker.ts index ea87cd1e91..a72acf34db 100644 --- a/compiler/packages/snap/src/runner-worker.ts +++ b/compiler/packages/snap/src/runner-worker.ts @@ -12,16 +12,19 @@ import type {printFunctionWithOutlined as PrintFunctionWithOutlined} from 'babel import type {printReactiveFunctionWithOutlined as PrintReactiveFunctionWithOutlined} from 'babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction'; import {TransformResult, transformFixtureInput} from './compiler'; import { - COMPILER_PATH, - COMPILER_INDEX_PATH, - PARSE_CONFIG_PRAGMA_PATH, - PRINT_HIR_PATH, - PRINT_REACTIVE_IR_PATH, + PARSE_CONFIG_PRAGMA_IMPORT, + PRINT_HIR_IMPORT, + PRINT_REACTIVE_IR_IMPORT, + PROJECT_SRC, } 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 type { + CompilerPipelineValue, + Effect, + ValueKind, +} from 'babel-plugin-react-compiler/src'; import chalk from 'chalk'; const originalConsoleError = console.error; @@ -61,22 +64,29 @@ async function compile( let compileResult: TransformResult | null = null; let error: string | null = null; try { + const importedCompilerPlugin = require(PROJECT_SRC) as Record< + string, + unknown + >; + // NOTE: we intentionally require lazily here so that we can clear the require cache // and load fresh versions of the compiler when `compilerVersion` changes. - const {default: BabelPluginReactCompiler} = require(COMPILER_PATH) as { - default: PluginObj; - }; - const {Effect: EffectEnum, ValueKind: ValueKindEnum} = require( - COMPILER_INDEX_PATH, - ); - const {printFunctionWithOutlined} = require(PRINT_HIR_PATH) as { - printFunctionWithOutlined: typeof PrintFunctionWithOutlined; - }; - const {printReactiveFunctionWithOutlined} = require( - PRINT_REACTIVE_IR_PATH, - ) as { - printReactiveFunctionWithOutlined: typeof PrintReactiveFunctionWithOutlined; - }; + const BabelPluginReactCompiler = importedCompilerPlugin[ + 'default' + ] as PluginObj; + const EffectEnum = importedCompilerPlugin['Effect'] as typeof Effect; + const ValueKindEnum = importedCompilerPlugin[ + 'ValueKind' + ] as typeof ValueKind; + const printFunctionWithOutlined = importedCompilerPlugin[ + PRINT_HIR_IMPORT + ] as typeof PrintFunctionWithOutlined; + const printReactiveFunctionWithOutlined = importedCompilerPlugin[ + PRINT_REACTIVE_IR_IMPORT + ] as typeof PrintReactiveFunctionWithOutlined; + const parseConfigPragmaForTests = importedCompilerPlugin[ + PARSE_CONFIG_PRAGMA_IMPORT + ] as typeof ParseConfigPragma; let lastLogged: string | null = null; const debugIRLogger = shouldLog @@ -106,9 +116,6 @@ async function compile( } } : () => {}; - 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 diff --git a/compiler/packages/snap/src/runner.ts b/compiler/packages/snap/src/runner.ts index cb4157bdd8..d46a18712e 100644 --- a/compiler/packages/snap/src/runner.ts +++ b/compiler/packages/snap/src/runner.ts @@ -12,7 +12,7 @@ import * as readline from 'readline'; import ts from 'typescript'; import yargs from 'yargs'; import {hideBin} from 'yargs/helpers'; -import {FILTER_PATH} from './constants'; +import {FILTER_PATH, PROJECT_ROOT} from './constants'; import {TestFilter, getFixtures, readTestFilter} from './fixture-utils'; import {TestResult, TestResults, report, update} from './reporter'; import { @@ -22,6 +22,7 @@ import { watchSrc, } from './runner-watch'; import * as runnerWorker from './runner-worker'; +import {execSync} from 'child_process'; const WORKER_PATH = require.resolve('./runner-worker.js'); const NUM_WORKERS = cpus().length - 1; @@ -205,23 +206,29 @@ export async function main(opts: RunnerOptions): Promise { const tsWatch: ts.WatchOfConfigFile = watchSrc( () => {}, - async (compileSuccess: boolean) => { - let isSuccess = compileSuccess; - if (compileSuccess) { - const testFilter = opts.filter ? await readTestFilter() : null; - const results = await runFixtures(worker, testFilter, 0); - if (opts.update) { - update(results); - } else { - const testSuccess = report(results); - isSuccess &&= testSuccess; - } - } else { + async (isTypecheckSuccess: boolean) => { + let isSuccess = false; + if (!isTypecheckSuccess) { console.error( - 'Found errors in Forget source code, skipping test fixtures.', + 'Found typescript errors in Forget source code, skipping test fixtures.', ); + } else { + try { + execSync('yarn build', {cwd: PROJECT_ROOT}); + console.log('Built compiler successfully with tsup'); + const testFilter = opts.filter ? await readTestFilter() : null; + const results = await runFixtures(worker, testFilter, 0); + if (opts.update) { + update(results); + isSuccess = true; + } else { + isSuccess = report(results); + } + } catch (e) { + console.warn('Failed to build compiler with tsup:', e); + } } - tsWatch.close(); + tsWatch?.close(); await worker.end(); process.exit(isSuccess ? 0 : 1); }, diff --git a/compiler/yarn.lock b/compiler/yarn.lock index 16029bfc04..e93f5fa78b 100644 --- a/compiler/yarn.lock +++ b/compiler/yarn.lock @@ -657,7 +657,7 @@ js-tokens "^4.0.0" picocolors "^1.0.0" -"@babel/parser@^7.1.0", "@babel/parser@^7.14.7", "@babel/parser@^7.18.10", "@babel/parser@^7.19.1", "@babel/parser@^7.2.0": +"@babel/parser@^7.1.0", "@babel/parser@^7.14.7", "@babel/parser@^7.18.10", "@babel/parser@^7.2.0": version "7.19.1" resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.19.1.tgz#6f6d6c2e621aad19a92544cc217ed13f1aac5b4c" integrity sha512-h7RCSorm1DdTVGJf3P2Mhj3kdnkmF/EiysUkzS2TdgAYqyjFdMQJbVuXOBej2SBJaXan/lIVtT6KkGbyyq753A== @@ -667,6 +667,13 @@ resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.19.3.tgz#8dd36d17c53ff347f9e55c328710321b49479a9a" integrity sha512-pJ9xOlNWHiy9+FuFP09DEAFbAn4JskgRsVcc169w2xRBC3FRGuQEwjeIMMND9L2zc0iEhO/tGv4Zq+km+hxNpQ== +"@babel/parser@^7.20.15": + version "7.27.0" + resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.27.0.tgz#3d7d6ee268e41d2600091cbd4e145ffee85a44ec" + integrity sha512-iaepho73/2Pz7w2eMS0Q5f83+0RKI7i4xmiYeBmDzfRVbQtTOG7Ts0S4HzJVsTMGI9keU8rNfuZr8DKfSt7Yyg== + dependencies: + "@babel/types" "^7.27.0" + "@babel/parser@^7.20.7": version "7.21.2" resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.2.tgz#dacafadfc6d7654c3051a66d6fe55b6cb2f2a0b3" @@ -1696,7 +1703,7 @@ debug "^4.3.1" globals "^11.1.0" -"@babel/types@7.26.3", "@babel/types@^7.0.0", "@babel/types@^7.18.10", "@babel/types@^7.18.6", "@babel/types@^7.18.9", "@babel/types@^7.19.0", "@babel/types@^7.2.0", "@babel/types@^7.2.2", "@babel/types@^7.20.0", "@babel/types@^7.20.2", "@babel/types@^7.20.5", "@babel/types@^7.20.7", "@babel/types@^7.21.2", "@babel/types@^7.21.4", "@babel/types@^7.21.5", "@babel/types@^7.22.0", "@babel/types@^7.22.3", "@babel/types@^7.22.4", "@babel/types@^7.22.5", "@babel/types@^7.24.7", "@babel/types@^7.25.0", "@babel/types@^7.25.6", "@babel/types@^7.25.9", "@babel/types@^7.26.0", "@babel/types@^7.26.10", "@babel/types@^7.26.3", "@babel/types@^7.26.9", "@babel/types@^7.3.0", "@babel/types@^7.3.3", "@babel/types@^7.4.4", "@babel/types@^7.7.4": +"@babel/types@7.26.3", "@babel/types@^7.0.0", "@babel/types@^7.18.10", "@babel/types@^7.18.6", "@babel/types@^7.18.9", "@babel/types@^7.19.0", "@babel/types@^7.2.0", "@babel/types@^7.2.2", "@babel/types@^7.20.0", "@babel/types@^7.20.2", "@babel/types@^7.20.5", "@babel/types@^7.20.7", "@babel/types@^7.21.2", "@babel/types@^7.21.4", "@babel/types@^7.21.5", "@babel/types@^7.22.0", "@babel/types@^7.22.3", "@babel/types@^7.22.4", "@babel/types@^7.22.5", "@babel/types@^7.24.7", "@babel/types@^7.25.0", "@babel/types@^7.25.6", "@babel/types@^7.25.9", "@babel/types@^7.26.0", "@babel/types@^7.26.10", "@babel/types@^7.26.3", "@babel/types@^7.26.9", "@babel/types@^7.27.0", "@babel/types@^7.3.0", "@babel/types@^7.3.3", "@babel/types@^7.4.4", "@babel/types@^7.7.4": version "7.26.3" resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.26.3.tgz#37e79830f04c2b5687acc77db97fbc75fb81f3c0" integrity sha512-vN5p+1kl59GVKMvTHt55NzzmYVxprfJD+ql7U9NFIfKCBkYE55LYtS+WtPlaYOyzydrKI8Nezd+aZextrd+FMA== From 254114616a24e0ed66468570b00d34bfabf9f73b Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Wed, 26 Mar 2025 14:31:20 -0400 Subject: [PATCH 03/13] [compiler][be] Playground now uses tsup bundled plugin (#32759) Followup to https://github.com/facebook/react/pull/32758. This moves playground to use the tsup bundled plugin instead of webpack-built `babel-plugin-react-compiler`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32759). * __->__ #32759 * #32758 --- compiler/apps/playground/components/Editor/EditorImpl.tsx | 6 +++--- compiler/apps/playground/components/Editor/Input.tsx | 2 +- compiler/apps/playground/components/Editor/Output.tsx | 2 +- .../apps/playground/lib/reactCompilerMonacoDiagnostics.ts | 7 ++----- compiler/apps/playground/package.json | 2 +- compiler/packages/babel-plugin-react-compiler/package.json | 2 +- 6 files changed, 9 insertions(+), 12 deletions(-) diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index 8c38611686..39571fa092 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -19,7 +19,9 @@ import BabelPluginReactCompiler, { PluginOptions, CompilerPipelineValue, parsePluginOptions, -} from 'babel-plugin-react-compiler/src'; + printReactiveFunctionWithOutlined, + printFunctionWithOutlined, +} from 'babel-plugin-react-compiler'; import clsx from 'clsx'; import invariant from 'invariant'; import {useSnackbar} from 'notistack'; @@ -41,8 +43,6 @@ import { 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'; function parseInput( diff --git a/compiler/apps/playground/components/Editor/Input.tsx b/compiler/apps/playground/components/Editor/Input.tsx index 2adfdb512f..0992591183 100644 --- a/compiler/apps/playground/components/Editor/Input.tsx +++ b/compiler/apps/playground/components/Editor/Input.tsx @@ -6,7 +6,7 @@ */ import MonacoEditor, {loader, type Monaco} from '@monaco-editor/react'; -import {CompilerErrorDetail} from 'babel-plugin-react-compiler/src'; +import {CompilerErrorDetail} from 'babel-plugin-react-compiler'; import invariant from 'invariant'; import type {editor} from 'monaco-editor'; import * as monaco from 'monaco-editor'; diff --git a/compiler/apps/playground/components/Editor/Output.tsx b/compiler/apps/playground/components/Editor/Output.tsx index d4127c63cf..7886f11e62 100644 --- a/compiler/apps/playground/components/Editor/Output.tsx +++ b/compiler/apps/playground/components/Editor/Output.tsx @@ -11,7 +11,7 @@ import { InformationCircleIcon, } from '@heroicons/react/outline'; import MonacoEditor, {DiffEditor} from '@monaco-editor/react'; -import {type CompilerError} from 'babel-plugin-react-compiler/src'; +import {type CompilerError} from 'babel-plugin-react-compiler'; import parserBabel from 'prettier/plugins/babel'; import * as prettierPluginEstree from 'prettier/plugins/estree'; import * as prettier from 'prettier/standalone'; diff --git a/compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts b/compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts index 76bcc5da37..a800e25773 100644 --- a/compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts +++ b/compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts @@ -6,10 +6,7 @@ */ import {Monaco} from '@monaco-editor/react'; -import { - CompilerErrorDetail, - ErrorSeverity, -} from 'babel-plugin-react-compiler/src'; +import {CompilerErrorDetail, ErrorSeverity} from 'babel-plugin-react-compiler'; import {MarkerSeverity, type editor} from 'monaco-editor'; function mapReactCompilerSeverityToMonaco( @@ -54,7 +51,7 @@ export function renderReactCompilerMarkers({ model, details, }: ReactCompilerMarkerConfig): void { - let markers = []; + const markers: Array = []; for (const detail of details) { const marker = mapReactCompilerDiagnosticToMonacoMarker(detail, monaco); if (marker == null) { diff --git a/compiler/apps/playground/package.json b/compiler/apps/playground/package.json index 795e9525ce..6e4ee9de87 100644 --- a/compiler/apps/playground/package.json +++ b/compiler/apps/playground/package.json @@ -4,7 +4,7 @@ "private": true, "scripts": { "dev": "cd ../.. && concurrently --kill-others -n compiler,runtime,playground \"yarn workspace babel-plugin-react-compiler run watch\" \"yarn workspace react-compiler-runtime run watch\" \"wait-on packages/babel-plugin-react-compiler/dist/index.js && cd apps/playground && NODE_ENV=development next dev\"", - "build:compiler": "cd ../.. && concurrently -n compiler,runtime \"yarn workspace babel-plugin-react-compiler run build\" \"yarn workspace react-compiler-runtime run build\"", + "build:compiler": "cd ../.. && concurrently -n compiler,runtime \"yarn workspace babel-plugin-react-compiler run build --dts\" \"yarn workspace react-compiler-runtime run build\"", "build": "yarn build:compiler && next build", "postbuild": "node ./scripts/downloadFonts.js", "preinstall": "cd ../.. && yarn install --frozen-lockfile", diff --git a/compiler/packages/babel-plugin-react-compiler/package.json b/compiler/packages/babel-plugin-react-compiler/package.json index f3ded80a49..75cf3ba53c 100644 --- a/compiler/packages/babel-plugin-react-compiler/package.json +++ b/compiler/packages/babel-plugin-react-compiler/package.json @@ -17,7 +17,7 @@ "snap:ci": "yarn snap:build && yarn snap", "ts:analyze-trace": "scripts/ts-analyze-trace.sh", "lint": "yarn eslint src", - "watch": "yarn build --watch" + "watch": "yarn build --dts --watch" }, "dependencies": { "@babel/types": "^7.26.0" From a5297ece6217f5495cbe38ba58f928b2697b0f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 26 Mar 2025 14:40:23 -0400 Subject: [PATCH 04/13] Don't flush synchronous work if we're in the middle of a ViewTransition async sequence (#32760) Starting a View Transition is an async sequence. Since React can get a sync update in the middle of sequence we sometimes interrupt that sequence. Currently, we don't actually cancel the View Transition so it can just run as a partial. This ensures that we fully skip it when that happens, as well as warn. However, it's very easy to trigger this with just a setState in useLayoutEffect right now. Therefore if we're inside the preparing sequence of a startViewTransition, this delays work that would've normally flushed in a microtask. ~Maybe we want to do the same for Default work already scheduled through a scheduler Task.~ Edit: This was already done. `flushSync` currently will still lead to an interrupted View Transition (with a warning). There's a tradeoff here whether we want to try our best to preserve the guarantees of `flushSync` or favor the animation. It's already possible to suspend at the root with `flushSync` which means it's not always 100% guaranteed to commit anyway. We could treat it as suspended. But let's see how much this is a problem in practice. --- .../view-transition/src/components/Page.js | 11 +++++ packages/react-art/src/ReactFiberConfigART.js | 10 +++-- .../src/client/ReactFiberConfigDOM.js | 18 +++++--- .../src/ReactFiberConfigNative.js | 15 ++++--- .../src/createReactNoop.js | 18 +++++--- .../src/ReactFiberConfigWithNoMutation.js | 4 +- .../src/ReactFiberGestureScheduler.js | 13 +++--- .../src/ReactFiberRootScheduler.js | 7 +++- .../src/ReactFiberWorkLoop.js | 41 +++++++++++++++---- .../src/forks/ReactFiberConfig.custom.js | 4 +- .../src/ReactFiberConfigTestHost.js | 15 ++++--- 11 files changed, 110 insertions(+), 46 deletions(-) diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index ee4b95331f..e51beeec0d 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -2,6 +2,7 @@ import React, { unstable_ViewTransition as ViewTransition, unstable_Activity as Activity, unstable_useSwipeTransition as useSwipeTransition, + useLayoutEffect, useEffect, useState, useId, @@ -68,6 +69,16 @@ export default function Page({url, navigate}) { return () => clearInterval(timer); }, []); + useLayoutEffect(() => { + // Calling a default update should not interrupt ViewTransitions but + // a flushSync will. + // Promise.resolve().then(() => { + // flushSync(() => { + setCounter(c => c + 10); + // }); + // }); + }, [show]); + const exclamation = ( ! diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index a168975221..e9d18a3081 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -538,14 +538,16 @@ export function hasInstanceAffectedParent( } export function startViewTransition() { - return false; + return null; } -export type RunningGestureTransition = null; +export type RunningViewTransition = null; -export function startGestureTransition() {} +export function startGestureTransition() { + return null; +} -export function stopGestureTransition(transition: RunningGestureTransition) {} +export function stopViewTransition(transition: RunningViewTransition) {} export type ViewTransitionInstance = null | {name: string, ...}; diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index f01817ce24..f3f458e3bd 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -1687,7 +1687,7 @@ export function startViewTransition( spawnedWorkCallback: () => void, passiveCallback: () => mixed, errorCallback: mixed => void, -): boolean { +): null | RunningViewTransition { const ownerDocument: Document = rootContainer.nodeType === DOCUMENT_NODE ? (rootContainer: any) @@ -1764,7 +1764,7 @@ export function startViewTransition( } passiveCallback(); }); - return true; + return transition; } catch (x) { // We use the error as feature detection. // The only thing that should throw is if startViewTransition is missing @@ -1772,11 +1772,17 @@ export function startViewTransition( // I.e. it's before the View Transitions v2 spec. We only support View // Transitions v2 otherwise we fallback to not animating to ensure that // we're not animating with the wrong animation mapped. - return false; + // Flush remaining work synchronously. + mutationCallback(); + layoutCallback(); + // Skip afterMutationCallback(). We don't need it since we're not animating. + spawnedWorkCallback(); + // Skip passiveCallback(). Spawned work will schedule a task. + return null; } } -export type RunningGestureTransition = { +export type RunningViewTransition = { skipTransition(): void, ... }; @@ -1900,7 +1906,7 @@ export function startGestureTransition( mutationCallback: () => void, animateCallback: () => void, errorCallback: mixed => void, -): null | RunningGestureTransition { +): null | RunningViewTransition { const ownerDocument: Document = rootContainer.nodeType === DOCUMENT_NODE ? (rootContainer: any) @@ -2072,7 +2078,7 @@ export function startGestureTransition( } } -export function stopGestureTransition(transition: RunningGestureTransition) { +export function stopViewTransition(transition: RunningViewTransition) { transition.skipTransition(); } diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index b15a6f9f76..876540a42d 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -653,11 +653,16 @@ export function startViewTransition( spawnedWorkCallback: () => void, passiveCallback: () => mixed, errorCallback: mixed => void, -): boolean { - return false; +): null | RunningViewTransition { + mutationCallback(); + layoutCallback(); + // Skip afterMutationCallback(). We don't need it since we're not animating. + spawnedWorkCallback(); + // Skip passiveCallback(). Spawned work will schedule a task. + return null; } -export type RunningGestureTransition = null; +export type RunningViewTransition = null; export function startGestureTransition( rootContainer: Container, @@ -668,13 +673,13 @@ export function startGestureTransition( mutationCallback: () => void, animateCallback: () => void, errorCallback: mixed => void, -): RunningGestureTransition { +): null | RunningViewTransition { mutationCallback(); animateCallback(); return null; } -export function stopGestureTransition(transition: RunningGestureTransition) {} +export function stopViewTransition(transition: RunningViewTransition) {} export type ViewTransitionInstance = null | {name: string, ...}; diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index efb5f955ca..d452e8e10e 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -93,7 +93,7 @@ export type TransitionStatus = mixed; export type FormInstance = Instance; -export type RunningGestureTransition = null; +export type RunningViewTransition = null; export type ViewTransitionInstance = null | {name: string, ...}; @@ -826,12 +826,18 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { rootContainer: Container, transitionTypes: null | TransitionTypes, mutationCallback: () => void, - afterMutationCallback: () => void, layoutCallback: () => void, + afterMutationCallback: () => void, + spawnedWorkCallback: () => void, passiveCallback: () => mixed, errorCallback: mixed => void, - ): boolean { - return false; + ): null | RunningViewTransition { + mutationCallback(); + layoutCallback(); + // Skip afterMutationCallback(). We don't need it since we're not animating. + spawnedWorkCallback(); + // Skip passiveCallback(). Spawned work will schedule a task. + return null; }, startGestureTransition( @@ -843,13 +849,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { mutationCallback: () => void, animateCallback: () => void, errorCallback: mixed => void, - ): RunningGestureTransition { + ): null | RunningViewTransition { mutationCallback(); animateCallback(); return null; }, - stopGestureTransition(transition: RunningGestureTransition) {}, + stopViewTransition(transition: RunningViewTransition) {}, createViewTransitionInstance(name: string): ViewTransitionInstance { return null; diff --git a/packages/react-reconciler/src/ReactFiberConfigWithNoMutation.js b/packages/react-reconciler/src/ReactFiberConfigWithNoMutation.js index 74e30da88c..bb347defe8 100644 --- a/packages/react-reconciler/src/ReactFiberConfigWithNoMutation.js +++ b/packages/react-reconciler/src/ReactFiberConfigWithNoMutation.js @@ -51,9 +51,9 @@ export const wasInstanceInViewport = shim; export const hasInstanceChanged = shim; export const hasInstanceAffectedParent = shim; export const startViewTransition = shim; -export type RunningGestureTransition = null; +export type RunningViewTransition = null; export const startGestureTransition = shim; -export const stopGestureTransition = shim; +export const stopViewTransition = shim; export type ViewTransitionInstance = null | {name: string, ...}; export const createViewTransitionInstance = shim; export type GestureTimeline = any; diff --git a/packages/react-reconciler/src/ReactFiberGestureScheduler.js b/packages/react-reconciler/src/ReactFiberGestureScheduler.js index 33d477f07d..18887528c0 100644 --- a/packages/react-reconciler/src/ReactFiberGestureScheduler.js +++ b/packages/react-reconciler/src/ReactFiberGestureScheduler.js @@ -8,10 +8,7 @@ */ import type {FiberRoot} from './ReactInternalTypes'; -import type { - GestureTimeline, - RunningGestureTransition, -} from './ReactFiberConfig'; +import type {GestureTimeline, RunningViewTransition} from './ReactFiberConfig'; import { GestureLane, @@ -21,7 +18,7 @@ import { import {ensureRootIsScheduled} from './ReactFiberRootScheduler'; import { subscribeToGestureDirection, - stopGestureTransition, + stopViewTransition, } from './ReactFiberConfig'; // This type keeps track of any scheduled or active gestures. @@ -33,7 +30,7 @@ export type ScheduledGesture = { rangeCurrent: number, // The starting offset along the timeline. rangeNext: number, // The end along the timeline where the next state is reached. cancel: () => void, // Cancel the subscription to direction change. - running: null | RunningGestureTransition, // Used to cancel the running transition after we're done. + running: null | RunningViewTransition, // Used to cancel the running transition after we're done. prev: null | ScheduledGesture, // The previous scheduled gesture in the queue for this root. next: null | ScheduledGesture, // The next scheduled gesture in the queue for this root. }; @@ -144,7 +141,7 @@ export function cancelScheduledGesture( } else { gesture.running = null; // If there's no work scheduled so we can stop the View Transition right away. - stopGestureTransition(runningTransition); + stopViewTransition(runningTransition); } } } @@ -183,7 +180,7 @@ export function stopCompletedGestures(root: FiberRoot) { root.stoppingGestures = null; while (gesture !== null) { if (gesture.running !== null) { - stopGestureTransition(gesture.running); + stopViewTransition(gesture.running); gesture.running = null; } const nextGesture = gesture.next; diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 293992e406..e7e61fb6cd 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -310,7 +310,12 @@ function processRootScheduleInMicrotask() { // At the end of the microtask, flush any pending synchronous work. This has // to come at the end, because it does actual rendering work that might throw. - flushSyncWorkAcrossRoots_impl(syncTransitionLanes, false); + // If we're in the middle of a View Transition async sequence, we don't want to + // interrupt that sequence. Instead, we'll flush any remaining work when it + // completes. + if (!hasPendingCommitEffects()) { + flushSyncWorkAcrossRoots_impl(syncTransitionLanes, false); + } } function scheduleTaskForRootDuringMicrotask( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 3da001f34c..c86a5f084e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -21,7 +21,11 @@ import type { TransitionAbort, } from './ReactFiberTracingMarkerComponent'; import type {OffscreenInstance} from './ReactFiberActivityComponent'; -import type {Resource, ViewTransitionInstance} from './ReactFiberConfig'; +import type { + Resource, + ViewTransitionInstance, + RunningViewTransition, +} from './ReactFiberConfig'; import type {RootState} from './ReactFiberRoot'; import { getViewTransitionName, @@ -102,6 +106,7 @@ import { trackSchedulerEvent, startViewTransition, startGestureTransition, + stopViewTransition, createViewTransitionInstance, } from './ReactFiberConfig'; @@ -665,6 +670,7 @@ let pendingEffectsRemainingLanes: Lanes = NoLanes; let pendingEffectsRenderEndTime: number = -0; // Profiling-only let pendingPassiveTransitions: Array | null = null; let pendingRecoverableErrors: null | Array> = null; +let pendingViewTransition: null | RunningViewTransition = null; let pendingViewTransitionEvents: Array<(types: Array) => void> | null = null; let pendingTransitionTypes: null | TransitionTypes = null; @@ -3503,10 +3509,8 @@ function commitRoot( } pendingEffectsStatus = PENDING_MUTATION_PHASE; - const startedViewTransition = - enableViewTransition && - willStartViewTransition && - startViewTransition( + if (enableViewTransition && willStartViewTransition) { + pendingViewTransition = startViewTransition( root.containerInfo, pendingTransitionTypes, flushMutationEffects, @@ -3516,7 +3520,7 @@ function commitRoot( flushPassiveEffects, reportViewTransitionError, ); - if (!startedViewTransition) { + } else { // Flush synchronously. flushMutationEffects(); flushLayoutEffects(); @@ -3646,6 +3650,8 @@ function flushSpawnedWork(): void { } pendingEffectsStatus = NO_PENDING_EFFECTS; + pendingViewTransition = null; // The view transition has now fully started. + // Tell Scheduler to yield at the end of the frame, so the browser has an // opportunity to paint. requestPaint(); @@ -3915,7 +3921,7 @@ function commitGestureOnRoot( pendingTransitionTypes = null; pendingEffectsStatus = PENDING_GESTURE_MUTATION_PHASE; - finishedGesture.running = startGestureTransition( + pendingViewTransition = finishedGesture.running = startGestureTransition( root.containerInfo, finishedGesture.provider, finishedGesture.rangeCurrent, @@ -3975,6 +3981,8 @@ function flushGestureAnimations(): void { pendingFinishedWork = (null: any); // Clear for GC purposes. pendingEffectsLanes = NoLanes; + pendingViewTransition = null; // The view transition has now fully started. + const prevTransition = ReactSharedInternals.T; ReactSharedInternals.T = null; const previousPriority = getCurrentUpdatePriority(); @@ -4025,8 +4033,27 @@ function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) { } } +let didWarnAboutInterruptedViewTransitions = false; + export function flushPendingEffects(wasDelayedCommit?: boolean): boolean { // Returns whether passive effects were flushed. + if (enableViewTransition && pendingViewTransition !== null) { + // If we forced a flush before the View Transition full started then we skip it. + // This ensures that we're not running a partial animation. + stopViewTransition(pendingViewTransition); + if (__DEV__) { + if (!didWarnAboutInterruptedViewTransitions) { + didWarnAboutInterruptedViewTransitions = true; + console.warn( + 'A flushSync update cancelled a View Transition because it was called ' + + 'while the View Transition was still preparing. To preserve the synchronous ' + + 'semantics, React had to skip the View Transition. If you can, try to avoid ' + + "flushSync() in a scenario that's likely to interfere.", + ); + } + } + pendingViewTransition = null; + } flushGestureMutations(); flushGestureAnimations(); flushMutationEffects(); diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index f22b6a580e..b4a025678b 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -40,7 +40,7 @@ export opaque type NoTimeout = mixed; export opaque type RendererInspectionConfig = mixed; export opaque type TransitionStatus = mixed; export opaque type FormInstance = mixed; -export type RunningGestureTransition = mixed; +export type RunningViewTransition = mixed; export type ViewTransitionInstance = null | {name: string, ...}; export opaque type InstanceMeasurement = mixed; export type EventResponder = any; @@ -155,7 +155,7 @@ export const hasInstanceChanged = $$$config.hasInstanceChanged; export const hasInstanceAffectedParent = $$$config.hasInstanceAffectedParent; export const startViewTransition = $$$config.startViewTransition; export const startGestureTransition = $$$config.startGestureTransition; -export const stopGestureTransition = $$$config.stopGestureTransition; +export const stopViewTransition = $$$config.stopViewTransition; export const getCurrentGestureOffset = $$$config.getCurrentGestureOffset; export const subscribeToGestureDirection = $$$config.subscribeToGestureDirection; diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 2df5f01571..f3701d3063 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -422,11 +422,16 @@ export function startViewTransition( spawnedWorkCallback: () => void, passiveCallback: () => mixed, errorCallback: mixed => void, -): boolean { - return false; +): null | RunningViewTransition { + mutationCallback(); + layoutCallback(); + // Skip afterMutationCallback(). We don't need it since we're not animating. + spawnedWorkCallback(); + // Skip passiveCallback(). Spawned work will schedule a task. + return null; } -export type RunningGestureTransition = null; +export type RunningViewTransition = null; export function startGestureTransition( rootContainer: Container, @@ -437,13 +442,13 @@ export function startGestureTransition( mutationCallback: () => void, animateCallback: () => void, errorCallback: mixed => void, -): RunningGestureTransition { +): null | RunningViewTransition { mutationCallback(); animateCallback(); return null; } -export function stopGestureTransition(transition: RunningGestureTransition) {} +export function stopViewTransition(transition: RunningViewTransition) {} export type ViewTransitionInstance = null | {name: string, ...}; From e0c99c4ea1cae566ad8040180cf180ae058cb8bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 26 Mar 2025 15:02:05 -0400 Subject: [PATCH 05/13] Rename to (#32734) It was always confusing that this is not a CSS class but a view-transition-class. The `className` sticks out a bit among its siblings `enter`, `exit`, `update` and `share`. The idea is that the most specific definition override is the class name that gets applied and this prop is really just the fallback, catch-all or "any" that is applied if you didn't specify a more specific one. It has also since evolved not just to take a string but also a map of Transition Type to strings. The "class" is really the type of the value. We could add a suffix to all of them like `defaultClass`, `enterClass`, `exitClass`, `updateClass` and `shareClass`. However, this doesn't necessarily make sense with the mapping of Transition Type to string. It also makes it a bit too DOM centric. In React Native this might still be called a "class" but it might be represented by an object definition. We might even allow some kind of inline style form for the DOM too. Really this is about picking which "animation" that runs which can be a string or instance. "Animation" is too broad because there's also a concept of a CSS Animation and these are really sets of CSS animations (group, image-pair, old, new). It could maybe be `defaultTransition`, `enterTransition`, etc but that seems unnecessarily repetitive and still doesn't say anything about it being a class. We also already have the name "default" in the map of Transition Types. In fact you can now specify a default for default: ``` ``` One thing I don't like about the name `"default"` is that it might be common to just apply a named class that does it matching to enter/exit/update in the CSS selectors (such as the `:only-child` rule) instead of doing that mapping to each one using React. In that can you end up specifying only `default={...}` a lot and then what is it the "default" for? It's more like "all". I think it's likely that you end up with either "default" or the specific forms instead of both at once. --- .../view-transition/src/components/Page.js | 8 +++---- .../src/ReactFiberApplyGesture.js | 12 +++++------ .../src/ReactFiberBeginWork.js | 21 +++++++++++++++++++ .../src/ReactFiberCommitViewTransitions.js | 16 +++++++------- .../src/ReactFiberViewTransitionComponent.js | 8 +------ 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index e51beeec0d..d7b57c5110 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -33,7 +33,7 @@ const b = ( function Component() { return (

Slide In from Left, Slide Out to Right

@@ -97,17 +97,17 @@ export default function Page({url, navigate}) { }}> {url === '/?b' ? 'Goto A' : 'Goto B'} - +
- +

{!show ? 'A' : 'B' + counter}
diff --git a/packages/react-reconciler/src/ReactFiberApplyGesture.js b/packages/react-reconciler/src/ReactFiberApplyGesture.js index 42682e60c4..ee44233b8f 100644 --- a/packages/react-reconciler/src/ReactFiberApplyGesture.js +++ b/packages/react-reconciler/src/ReactFiberApplyGesture.js @@ -151,7 +151,7 @@ function trackDeletedPairViewTransitions(deletion: Fiber): void { // and can stop searching (size reaches zero). pairs.delete(name); const className: ?string = getViewTransitionClassName( - props.className, + props.default, props.share, ); if (className !== 'none') { @@ -196,7 +196,7 @@ function trackEnterViewTransitions(deletion: Fiber): void { ? appearingViewTransitions.get(name) : undefined; const className: ?string = getViewTransitionClassName( - props.className, + props.default, pair !== undefined ? props.share : props.enter, ); if (className !== 'none') { @@ -259,7 +259,7 @@ function applyAppearingPairViewTransition(child: Fiber): void { // Note that this class name that doesn't actually really matter because the // "new" side will be the one that wins in practice. const className: ?string = getViewTransitionClassName( - props.className, + props.default, props.share, ); if (className !== 'none') { @@ -282,7 +282,7 @@ function applyExitViewTransition(placement: Fiber): void { const props: ViewTransitionProps = placement.memoizedProps; const name = getViewTransitionName(props, state); const className: ?string = getViewTransitionClassName( - props.className, + props.default, // Note that just because we don't have a pair yet doesn't mean we won't find one // later. However, that doesn't matter because if we do the class name that wins // is the one applied by the "new" side anyway. @@ -307,7 +307,7 @@ function applyNestedViewTransition(child: Fiber): void { const props: ViewTransitionProps = child.memoizedProps; const name = getViewTransitionName(props, state); const className: ?string = getViewTransitionClassName( - props.className, + props.default, props.update, ); if (className !== 'none') { @@ -336,7 +336,7 @@ function applyUpdateViewTransition(current: Fiber, finishedWork: Fiber): void { // want the props from "current" since that's the class that would've won if // it was the normal direction. To preserve the same effect in either direction. const className: ?string = getViewTransitionClassName( - newProps.className, + newProps.default, newProps.update, ); if (className === 'none') { diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index a5ff0d1757..aa53e06ee6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -322,6 +322,7 @@ export let didWarnAboutReassigningProps: boolean; let didWarnAboutRevealOrder; let didWarnAboutTailOptions; let didWarnAboutDefaultPropsOnFunctionComponent; +let didWarnAboutClassNameOnViewTransition; if (__DEV__) { didWarnAboutBadClass = ({}: {[string]: boolean}); @@ -332,6 +333,7 @@ if (__DEV__) { didWarnAboutRevealOrder = ({}: {[empty]: boolean}); didWarnAboutTailOptions = ({}: {[string]: boolean}); didWarnAboutDefaultPropsOnFunctionComponent = ({}: {[string]: boolean}); + didWarnAboutClassNameOnViewTransition = ({}: {[string]: boolean}); } export function reconcileChildren( @@ -3295,6 +3297,25 @@ function updateViewTransition( pushMaterializedTreeId(workInProgress); } } + if (__DEV__) { + // $FlowFixMe[prop-missing] + if (pendingProps.className !== undefined) { + const example = + typeof pendingProps.className === 'string' + ? JSON.stringify(pendingProps.className) + : '{...}'; + if (!didWarnAboutClassNameOnViewTransition[example]) { + didWarnAboutClassNameOnViewTransition[example] = true; + console.error( + ' doesn\'t accept a "className" prop. It has been renamed to "default".\n' + + '- \n' + + '+ ', + example, + example, + ); + } + } + } if (current !== null && current.memoizedProps.name !== pendingProps.name) { // If the name changes, we schedule a ref effect to create a new ref instance. workInProgress.flags |= Ref | RefStatic; diff --git a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js index 2ebebc4e75..36948d7fd7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js +++ b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js @@ -228,7 +228,7 @@ function commitAppearingPairViewTransitions(placement: Fiber): void { } const name = props.name; const className: ?string = getViewTransitionClassName( - props.className, + props.default, props.share, ); if (className !== 'none') { @@ -267,7 +267,7 @@ export function commitEnterViewTransitions( const props: ViewTransitionProps = placement.memoizedProps; const name = getViewTransitionName(props, state); const className: ?string = getViewTransitionClassName( - props.className, + props.default, state.paired ? props.share : props.enter, ); if (className !== 'none') { @@ -337,7 +337,7 @@ function commitDeletedPairViewTransitions(deletion: Fiber): void { const pair = pairs.get(name); if (pair !== undefined) { const className: ?string = getViewTransitionClassName( - props.className, + props.default, props.share, ); if (className !== 'none') { @@ -389,7 +389,7 @@ export function commitExitViewTransitions(deletion: Fiber): void { ? appearingViewTransitions.get(name) : undefined; const className: ?string = getViewTransitionClassName( - props.className, + props.default, pair !== undefined ? props.share : props.exit, ); if (className !== 'none') { @@ -470,7 +470,7 @@ export function commitBeforeUpdateViewTransition( // a layout only change, then the "foo" class will be applied even though // it was not actually an update. Which is a bug. const className: ?string = getViewTransitionClassName( - newProps.className, + newProps.default, newProps.update, ); if (className === 'none') { @@ -495,7 +495,7 @@ export function commitNestedViewTransitions(changedParent: Fiber): void { const props: ViewTransitionProps = child.memoizedProps; const name = getViewTransitionName(props, child.stateNode); const className: ?string = getViewTransitionClassName( - props.className, + props.default, props.update, ); if (className !== 'none') { @@ -735,7 +735,7 @@ export function measureUpdateViewTransition( const oldName = getViewTransitionName(oldFiber.memoizedProps, state); // Whether it ends up having been updated or relayout we apply the update class name. const className: ?string = getViewTransitionClassName( - props.className, + props.default, props.update, ); if (className === 'none') { @@ -787,7 +787,7 @@ export function measureNestedViewTransitions( const state: ViewTransitionState = child.stateNode; const name = getViewTransitionName(props, state); const className: ?string = getViewTransitionClassName( - props.className, + props.default, props.update, ); let previousMeasurements: null | Array; diff --git a/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js b/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js index 3b5bede1a7..659029fb6b 100644 --- a/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js +++ b/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js @@ -29,7 +29,7 @@ export type ViewTransitionClass = 'none' | string | ViewTransitionClassPerType; export type ViewTransitionProps = { name?: string, children?: ReactNodeList, - className?: ViewTransitionClass, + default?: ViewTransitionClass, enter?: ViewTransitionClass, exit?: ViewTransitionClass, share?: ViewTransitionClass, @@ -129,11 +129,5 @@ export function getViewTransitionClassName( if (eventClassName == null) { return className; } - if (eventClassName === 'none') { - return eventClassName; - } - if (className != null && className !== 'none') { - return className + ' ' + eventClassName; - } return eventClassName; } From fceb0f80bc729d061bcb5031801cfc824adc07a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 26 Mar 2025 15:02:43 -0400 Subject: [PATCH 06/13] Add "auto" class to mean the built-in should run (#32761) Stacked on https://github.com/facebook/react/pull/32734 In React a ViewTransition class of `"none"` doesn't just mean that it has no class but also that it has no ViewTransition name. The default (`null | undefined`) means that it has no specific class but should run with the default built-in animation. This adds this as an explicit string called `"auto"` as well. That way you can do `` to override the "foo" just for the "enter" trigger to be the default built-in animation. Where as if you just specified `null` it would be like not specifying enter at all which would trigger "foo". --- .../src/ReactFiberViewTransitionComponent.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js b/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js index 659029fb6b..c4bc3204db 100644 --- a/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js +++ b/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js @@ -21,10 +21,14 @@ import {getIsHydrating} from './ReactFiberHydrationContext'; import {getTreeId} from './ReactFiberTreeContext'; export type ViewTransitionClassPerType = { - [transitionType: 'default' | string]: 'none' | string, + [transitionType: 'default' | string]: 'none' | 'auto' | string, }; -export type ViewTransitionClass = 'none' | string | ViewTransitionClassPerType; +export type ViewTransitionClass = + | 'none' + | 'auto' + | string + | ViewTransitionClassPerType; export type ViewTransitionProps = { name?: string, @@ -127,7 +131,10 @@ export function getViewTransitionClassName( const className: ?string = getClassNameByType(defaultClass); const eventClassName: ?string = getClassNameByType(eventClass); if (eventClassName == null) { - return className; + return className === 'auto' ? null : className; + } + if (eventClassName === 'auto') { + return null; } return eventClassName; } From f134b3993a84d53cc99fe66b426ba13548f142ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 26 Mar 2025 15:02:53 -0400 Subject: [PATCH 07/13] Add getComputedStyle helper to ViewTransition refs (#32751) This is also sometimes useful to read the style of the pseudo-element itself without an animation. --- .../react-dom-bindings/src/client/ReactFiberConfigDOM.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index f3f458e3bd..e6b54a1e5e 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2085,6 +2085,7 @@ export function stopViewTransition(transition: RunningViewTransition) { interface ViewTransitionPseudoElementType extends Animatable { _scope: HTMLElement; _selector: string; + getComputedStyle(): CSSStyleDeclaration; } function ViewTransitionPseudoElement( @@ -2138,6 +2139,14 @@ ViewTransitionPseudoElement.prototype.getAnimations = function ( } return result; }; +// $FlowFixMe[prop-missing] +ViewTransitionPseudoElement.prototype.getComputedStyle = function ( + this: ViewTransitionPseudoElementType, +): CSSStyleDeclaration { + const scope = this._scope; + const selector = this._selector; + return getComputedStyle(scope, selector); +}; export function createViewTransitionInstance( name: string, From 3e88e97c116c7a1535976f2d4486bbf345476443 Mon Sep 17 00:00:00 2001 From: Ricky Date: Wed, 26 Mar 2025 17:39:52 -0400 Subject: [PATCH 08/13] s/HTML/text for text hydration mismatches (#32763) --- .../react-dom/src/__tests__/ReactDOMFizzServer-test.js | 6 +++--- .../src/__tests__/ReactDOMHydrationDiff-test.js | 6 +++--- .../ReactDOMServerPartialHydration-test.internal.js | 4 ++-- .../react-dom/src/__tests__/ReactRenderDocument-test.js | 2 +- .../react-reconciler/src/ReactFiberHydrationContext.js | 9 +++++---- scripts/error-codes/codes.json | 2 +- 6 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 7542582528..48ffb10860 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4664,7 +4664,7 @@ describe('ReactDOMFizzServer', () => { // client-side rendering. await clientResolve(); await waitForAll([ - "onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.", + "onRecoverableError: Hydration failed because the server rendered text didn't match the client.", ]); expect(getVisibleChildren(container)).toEqual(
@@ -4712,7 +4712,7 @@ describe('ReactDOMFizzServer', () => { }, }); await waitForAll([ - "onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.", + "onRecoverableError: Hydration failed because the server rendered text didn't match the client.", ]); expect(getVisibleChildren(container)).toEqual( @@ -10179,7 +10179,7 @@ describe('ReactDOMFizzServer', () => { ); expect(recoverableErrors).toEqual([ expect.stringContaining( - "Hydration failed because the server rendered HTML didn't match the client.", + "Hydration failed because the server rendered text didn't match the client.", ), ]); } else { diff --git a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js index c445f458e5..8b0bb44ccf 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js @@ -127,7 +127,7 @@ describe('ReactDOMServerHydration', () => { if (gate(flags => flags.favorSafetyOverHydrationPerf)) { expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` [ - "Caught [Hydration failed because the server rendered HTML didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used: + "Caught [Hydration failed because the server rendered text didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used: - A server/client branch \`if (typeof window !== 'undefined')\`. - Variable input such as \`Date.now()\` or \`Math.random()\` which changes each time it's called. @@ -196,7 +196,7 @@ describe('ReactDOMServerHydration', () => { if (gate(flags => flags.favorSafetyOverHydrationPerf)) { expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` [ - "Caught [Hydration failed because the server rendered HTML didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used: + "Caught [Hydration failed because the server rendered text didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used: - A server/client branch \`if (typeof window !== 'undefined')\`. - Variable input such as \`Date.now()\` or \`Math.random()\` which changes each time it's called. @@ -743,7 +743,7 @@ describe('ReactDOMServerHydration', () => { if (gate(flags => flags.favorSafetyOverHydrationPerf)) { expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` [ - "Caught [Hydration failed because the server rendered HTML didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used: + "Caught [Hydration failed because the server rendered text didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used: - A server/client branch \`if (typeof window !== 'undefined')\`. - Variable input such as \`Date.now()\` or \`Math.random()\` which changes each time it's called. diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 5900a2f448..94d672cef4 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -3897,7 +3897,7 @@ describe('ReactDOMServerPartialHydration', () => { }); }); assertLog([ - "onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.", + "onRecoverableError: Hydration failed because the server rendered text didn't match the client.", ]); }); @@ -3936,7 +3936,7 @@ describe('ReactDOMServerPartialHydration', () => { ); }); assertLog([ - "onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.", + "onRecoverableError: Hydration failed because the server rendered text didn't match the client.", ]); }); }); diff --git a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js index 8395d2afde..9522a920bc 100644 --- a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js @@ -320,7 +320,7 @@ describe('rendering React components at document', () => { assertLog( favorSafetyOverHydrationPerf ? [ - "onRecoverableError: Hydration failed because the server rendered HTML didn't match the client.", + "onRecoverableError: Hydration failed because the server rendered text didn't match the client.", ] : [], ); diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index f6589b7445..c2507f3201 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -308,7 +308,7 @@ export const HydrationMismatchException: mixed = new Error( "userspace. If you're seeing this, it's likely a bug in React.", ); -function throwOnHydrationMismatch(fiber: Fiber) { +function throwOnHydrationMismatch(fiber: Fiber, fromText: boolean = false) { let diff = ''; if (__DEV__) { // Consume the diff root for this mismatch. @@ -320,7 +320,8 @@ function throwOnHydrationMismatch(fiber: Fiber) { } } const error = new Error( - "Hydration failed because the server rendered HTML didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used:\n" + + `Hydration failed because the server rendered ${fromText ? 'text' : 'HTML'} didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used: +` + '\n' + "- A server/client branch `if (typeof window !== 'undefined')`.\n" + "- Variable input such as `Date.now()` or `Math.random()` which changes each time it's called.\n" + @@ -481,7 +482,7 @@ function prepareToHydrateHostInstance( fiber, ); if (!didHydrate && favorSafetyOverHydrationPerf) { - throwOnHydrationMismatch(fiber); + throwOnHydrationMismatch(fiber, true); } } @@ -547,7 +548,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): void { parentProps, ); if (!didHydrate && favorSafetyOverHydrationPerf) { - throwOnHydrationMismatch(fiber); + throwOnHydrationMismatch(fiber, true); } } diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 9db8a9cb89..0bb0d5071a 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -403,7 +403,7 @@ "415": "Error parsing the data. It's probably an error code or network corruption.", "416": "This environment don't support binary chunks.", "417": "React currently only supports piping to one writable stream.", - "418": "Hydration failed because the server rendered HTML didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used:\n\n- A server/client branch `if (typeof window !== 'undefined')`.\n- Variable input such as `Date.now()` or `Math.random()` which changes each time it's called.\n- Date formatting in a user's locale which doesn't match the server.\n- External changing data without sending a snapshot of it along with the HTML.\n- Invalid HTML tag nesting.\n\nIt can also happen if the client has a browser extension installed which messes with the HTML before React loaded.\n\nhttps://react.dev/link/hydration-mismatch%s", + "418": "Hydration failed because the server rendered %s didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used:\n\n- A server/client branch `if (typeof window !== 'undefined')`.\n- Variable input such as `Date.now()` or `Math.random()` which changes each time it's called.\n- Date formatting in a user's locale which doesn't match the server.\n- External changing data without sending a snapshot of it along with the HTML.\n- Invalid HTML tag nesting.\n\nIt can also happen if the client has a browser extension installed which messes with the HTML before React loaded.\n\nhttps://react.dev/link/hydration-mismatch%s", "419": "The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.", "420": "ServerContext: %s already defined", "421": "This Suspense boundary received an update before it finished hydrating. This caused the boundary to switch to client rendering. The usual way to fix this is to wrap the original update in startTransition.", From 4280563b04898baad423dc7d0f8b0dfea3b1797a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 26 Mar 2025 18:12:59 -0400 Subject: [PATCH 09/13] Mark shouldStartViewTransition as true when there's an enter animation (#32764) Typically we mark the name of things that might animate in the snapshot phase. At the same time we track that should call startViewTransition too. However, we don't do this for "enter" since they're only marked later. Leading to having just an "enter" not to animate unless there's at least another update too. This tracks if there's a ViewTransitionComponent in the tree that enters. Luckily we know that from the static flag so we don't have to traverse it. --- .../src/ReactFiberCommitViewTransitions.js | 16 +++++++++++++++- .../react-reconciler/src/ReactFiberCommitWork.js | 7 +++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js index 36948d7fd7..969245da71 100644 --- a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js +++ b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js @@ -67,6 +67,20 @@ export function trackAppearingViewTransition( appearingViewTransitions.set(name, state); } +export function trackEnterViewTransitions(placement: Fiber): void { + if ( + placement.tag === ViewTransitionComponent || + (placement.subtreeFlags & ViewTransitionStatic) !== NoFlags + ) { + // If an inserted or appearing Fiber is a ViewTransition component or has one as + // an immediate child, then that will trigger as an "Enter" in future passes. + // We don't do anything else for that case in the "before mutation" phase but we + // still have to mark it as needing to call startViewTransition if nothing else + // updates. + shouldStartViewTransition = true; + } +} + // We can't cancel view transition children until we know that their parent also // don't need to transition. export let viewTransitionCancelableChildren: null | Array< @@ -119,7 +133,6 @@ function applyViewTransitionToHostInstancesRecursive( let inViewport = false; while (child !== null) { if (child.tag === HostComponent) { - shouldStartViewTransition = true; const instance: Instance = child.stateNode; if (collectMeasurements !== null) { const measurement = measureInstance(instance); @@ -132,6 +145,7 @@ function applyViewTransitionToHostInstancesRecursive( inViewport = true; } } + shouldStartViewTransition = true; applyViewTransitionName( instance, viewTransitionHostInstanceIdx === 0 diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 084a22e617..5b27c8e494 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -235,6 +235,7 @@ import { commitFragmentInstanceInsertionEffects, } from './ReactFiberCommitHostEffects'; import { + trackEnterViewTransitions, commitEnterViewTransitions, commitExitViewTransitions, commitBeforeUpdateViewTransition, @@ -338,6 +339,9 @@ function commitBeforeMutationEffects_begin(isViewTransitionEligible: boolean) { // to trigger updates of any nested view transitions and we shouldn't // have any other before mutation effects since snapshot effects are // only applied to updates. TODO: Model this using only flags. + if (isViewTransitionEligible) { + trackEnterViewTransitions(fiber); + } commitBeforeMutationEffects_complete(isViewTransitionEligible); continue; } @@ -367,6 +371,9 @@ function commitBeforeMutationEffects_begin(isViewTransitionEligible: boolean) { // to trigger updates of any nested view transitions and we shouldn't // have any other before mutation effects since snapshot effects are // only applied to updates. TODO: Model this using only flags. + if (isViewTransitionEligible) { + trackEnterViewTransitions(fiber); + } commitBeforeMutationEffects_complete(isViewTransitionEligible); continue; } From 8039f1b2a05d00437cd29707761aeae098c80adc Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Thu, 27 Mar 2025 12:18:50 -0400 Subject: [PATCH 10/13] [compiler] Fix inferEffectDependencies lint false positives (#32769) Currently, inferred effect dependencies are considered a "compiler-required" feature. This means that untransformed callsites should escalate to a build error. `ValidateNoUntransformedReferences` iterates 'special effect' callsites and checks that the compiler was able to successfully transform them. Prior to this PR, this relied on checking the number of arguments passed to this special effect. This obviously doesn't work with `noEmit: true`, which is used for our eslint plugin (this avoids mutating the babel program as other linters run with the same ast). This PR adds a set of `babel.SourceLocation`s to do best effort matching in this mode. --- .../src/Babel/BabelPlugin.ts | 2 +- .../src/Entrypoint/Program.ts | 15 +++++++-- .../ValidateNoUntransformedReferences.ts | 18 ++++++++--- .../src/HIR/Environment.ts | 8 +++++ .../src/Inference/InferEffectDependencies.ts | 2 ++ .../ReactiveScopes/CodegenReactiveFunction.ts | 2 ++ .../no-emit-lint-repro.expect.md | 31 +++++++++++++++++++ .../no-emit-lint-repro.js | 8 +++++ compiler/packages/snap/src/compiler.ts | 1 - 9 files changed, 78 insertions(+), 9 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.js 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 ff9817380f..5816719424 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts @@ -73,7 +73,7 @@ export default function BabelPluginReactCompiler( pass.filename ?? null, opts.logger, opts.environment, - result?.retryErrors ?? [], + result, ); if (ENABLE_REACT_COMPILER_TIMINGS === true) { performance.mark(`${filename}:end`, { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index c00c672b2c..622b7f72da 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -30,6 +30,7 @@ import { findProgramSuppressions, suppressionsToCompilerError, } from './Suppression'; +import {GeneratedSource} from '../HIR'; export type CompilerPass = { opts: PluginOptions; @@ -267,8 +268,9 @@ function isFilePartOfSources( return false; } -type CompileProgramResult = { +export type CompileProgramResult = { retryErrors: Array<{fn: BabelFn; error: CompilerError}>; + inferredEffectLocations: Set; }; /** * `compileProgram` is directly invoked by the react-compiler babel plugin, so @@ -369,6 +371,7 @@ export function compileProgram( }, ); const retryErrors: Array<{fn: BabelFn; error: CompilerError}> = []; + const inferredEffectLocations = new Set(); const processFn = ( fn: BabelFn, fnType: ReactFunctionType, @@ -509,6 +512,14 @@ export function compileProgram( if (!pass.opts.noEmit) { return compileResult.compiledFn; } + /** + * inferEffectDependencies + noEmit is currently only used for linting. In + * this mode, add source locations for where the compiler *can* infer effect + * dependencies. + */ + for (const loc of compileResult.compiledFn.inferredEffectLocations) { + if (loc !== GeneratedSource) inferredEffectLocations.add(loc); + } return null; }; @@ -587,7 +598,7 @@ export function compileProgram( if (compiledFns.length > 0) { addImportsToProgram(program, programContext); } - return {retryErrors}; + return {retryErrors, inferredEffectLocations}; } function shouldSkipCompilation( diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts index 07ab3b2b6a..a221b0485c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts @@ -11,6 +11,7 @@ import { import {getOrInsertWith} from '../Utils/utils'; import {Environment} from '../HIR'; import {DEFAULT_EXPORT} from '../HIR/Environment'; +import {CompileProgramResult} from './Program'; function throwInvalidReact( options: Omit, @@ -36,12 +37,16 @@ function assertValidEffectImportReference( const parent = path.parentPath; if (parent != null && parent.isCallExpression()) { const args = parent.get('arguments'); + const maybeCalleeLoc = path.node.loc; + const hasInferredEffect = + maybeCalleeLoc != null && + context.inferredEffectLocations.has(maybeCalleeLoc); /** * Only error on untransformed references of the form `useMyEffect(...)` * or `moduleNamespace.useMyEffect(...)`, with matching argument counts. * TODO: do we also want a mode to also hard error on non-call references? */ - if (args.length === numArgs) { + if (args.length === numArgs && !hasInferredEffect) { const maybeErrorDiagnostic = matchCompilerDiagnostic( path, context.transformErrors, @@ -97,7 +102,7 @@ export default function validateNoUntransformedReferences( filename: string | null, logger: Logger | null, env: EnvironmentConfig, - transformErrors: Array<{fn: NodePath; error: CompilerError}>, + compileResult: CompileProgramResult | null, ): void { const moduleLoadChecks = new Map< string, @@ -126,7 +131,7 @@ export default function validateNoUntransformedReferences( } } if (moduleLoadChecks.size > 0) { - transformProgram(path, moduleLoadChecks, filename, logger, transformErrors); + transformProgram(path, moduleLoadChecks, filename, logger, compileResult); } } @@ -136,6 +141,7 @@ type TraversalState = { logger: Logger | null; filename: string | null; transformErrors: Array<{fn: NodePath; error: CompilerError}>; + inferredEffectLocations: Set; }; type CheckInvalidReferenceFn = ( paths: Array>, @@ -223,14 +229,16 @@ function transformProgram( moduleLoadChecks: Map>, filename: string | null, logger: Logger | null, - transformErrors: Array<{fn: NodePath; error: CompilerError}>, + compileResult: CompileProgramResult | null, ): void { const traversalState: TraversalState = { shouldInvalidateScopes: true, program: path, filename, logger, - transformErrors, + transformErrors: compileResult?.retryErrors ?? [], + inferredEffectLocations: + compileResult?.inferredEffectLocations ?? new Set(), }; path.traverse({ ImportDeclaration(path: NodePath) { 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 2594ac31c6..276e4f7b40 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -11,6 +11,7 @@ import {fromZodError} from 'zod-validation-error'; import {CompilerError} from '../CompilerError'; import { CompilationMode, + defaultOptions, Logger, PanicThresholdOptions, parsePluginOptions, @@ -779,6 +780,7 @@ export function parseConfigPragmaForTests( const environment = parseConfigPragmaEnvironmentForTest(pragma); let compilationMode: CompilationMode = defaults.compilationMode; let panicThreshold: PanicThresholdOptions = 'all_errors'; + let noEmit: boolean = defaultOptions.noEmit; for (const token of pragma.split(' ')) { if (!token.startsWith('@')) { continue; @@ -804,12 +806,17 @@ export function parseConfigPragmaForTests( panicThreshold = 'none'; break; } + case '@noEmit': { + noEmit = true; + break; + } } } return parsePluginOptions({ environment, compilationMode, panicThreshold, + noEmit, }); } @@ -852,6 +859,7 @@ export class Environment { programContext: ProgramContext; hasFireRewrite: boolean; hasInferredEffect: boolean; + inferredEffectLocations: Set = new Set(); #contextIdentifiers: Set; #hoistedIdentifiers: Set; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 85cb023665..03bd9fd382 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -217,6 +217,7 @@ export function inferEffectDependencies(fn: HIRFunction): void { // Step 2: push the inferred deps array as an argument of the useEffect value.args.push({...depsPlace, effect: Effect.Freeze}); rewriteInstrs.set(instr.id, newInstructions); + fn.env.inferredEffectLocations.add(callee.loc); } else if (loadGlobals.has(value.args[0].identifier.id)) { // Global functions have no reactive dependencies, so we can insert an empty array newInstructions.push({ @@ -227,6 +228,7 @@ export function inferEffectDependencies(fn: HIRFunction): void { }); value.args.push({...depsPlace, effect: Effect.Freeze}); rewriteInstrs.set(instr.id, newInstructions); + fn.env.inferredEffectLocations.add(callee.loc); } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index b90e4e417c..9d41663d56 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -104,6 +104,7 @@ export type CodegenFunction = { * This is true if the compiler has compiled inferred effect dependencies */ hasInferredEffect: boolean; + inferredEffectLocations: Set; /** * This is true if the compiler has compiled a fire to a useFire call @@ -389,6 +390,7 @@ function codegenReactiveFunction( outlined: [], hasFireRewrite: fn.env.hasFireRewrite, hasInferredEffect: fn.env.hasInferredEffect, + inferredEffectLocations: fn.env.inferredEffectLocations, }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.expect.md new file mode 100644 index 0000000000..b5bbd032ba --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.expect.md @@ -0,0 +1,31 @@ + +## Input + +```javascript +// @inferEffectDependencies @noEmit +import {print} from 'shared-runtime'; +import useEffectWrapper from 'useEffectWrapper'; + +function ReactiveVariable({propVal}) { + const arr = [propVal]; + useEffectWrapper(() => print(arr)); +} + +``` + +## Code + +```javascript +// @inferEffectDependencies @noEmit +import { print } from "shared-runtime"; +import useEffectWrapper from "useEffectWrapper"; + +function ReactiveVariable({ propVal }) { + const arr = [propVal]; + useEffectWrapper(() => print(arr)); +} + +``` + +### 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/infer-effect-dependencies/no-emit-lint-repro.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.js new file mode 100644 index 0000000000..939b604530 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.js @@ -0,0 +1,8 @@ +// @inferEffectDependencies @noEmit +import {print} from 'shared-runtime'; +import useEffectWrapper from 'useEffectWrapper'; + +function ReactiveVariable({propVal}) { + const arr = [propVal]; + useEffectWrapper(() => print(arr)); +} diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 6e59276c1c..6fce644542 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -187,7 +187,6 @@ function makePluginOptions( }, logger, gating, - noEmit: false, eslintSuppressionRules, flowSuppressions, ignoreUseNoForget, From ef4bc8b4f91023afac437be9179beef350b32db3 Mon Sep 17 00:00:00 2001 From: Rodrigo Faria Date: Fri, 28 Mar 2025 15:10:32 +0000 Subject: [PATCH 11/13] feat(babel-plugin-react-compiler): support satisfies operator (#32742) Solve https://github.com/facebook/react/pull/29818 --------- Co-authored-by: Rodrigo Faria --- .../src/HIR/BuildHIR.ts | 14 ++++ .../src/HIR/HIR.ts | 14 +++- .../ReactiveScopes/CodegenReactiveFunction.ts | 15 ++-- .../type-annotation-satisfies-array.expect.md | 71 +++++++++++++++++++ .../type-annotation-satisfies-array.ts | 15 ++++ ...type-annotation-satisfies-number.expect.md | 41 +++++++++++ .../type-annotation-satisfies-number.ts | 13 ++++ 7 files changed, 176 insertions(+), 7 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-array.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-array.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-number.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-number.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index 6f93ef2f3a..cba4bf93ed 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -2406,6 +2406,19 @@ function lowerExpression( kind: 'TypeCastExpression', value: lowerExpressionToTemporary(builder, expr.get('expression')), typeAnnotation: typeAnnotation.node, + typeAnnotationKind: 'cast', + type: lowerType(typeAnnotation.node), + loc: exprLoc, + }; + } + case 'TSSatisfiesExpression': { + let expr = exprPath as NodePath; + const typeAnnotation = expr.get('typeAnnotation'); + return { + kind: 'TypeCastExpression', + value: lowerExpressionToTemporary(builder, expr.get('expression')), + typeAnnotation: typeAnnotation.node, + typeAnnotationKind: 'satisfies', type: lowerType(typeAnnotation.node), loc: exprLoc, }; @@ -2417,6 +2430,7 @@ function lowerExpression( kind: 'TypeCastExpression', value: lowerExpressionToTemporary(builder, expr.get('expression')), typeAnnotation: typeAnnotation.node, + typeAnnotationKind: 'as', type: lowerType(typeAnnotation.node), loc: exprLoc, }; 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 3a8cb89ca0..5c84cbb9fc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -910,13 +910,21 @@ export type InstructionValue = value: Place; loc: SourceLocation; } - | { + | ({ kind: 'TypeCastExpression'; value: Place; - typeAnnotation: t.FlowType | t.TSType; type: Type; loc: SourceLocation; - } + } & ( + | { + typeAnnotation: t.FlowType; + typeAnnotationKind: 'cast'; + } + | { + typeAnnotation: t.TSType; + typeAnnotationKind: 'as' | 'satisfies'; + } + )) | JsxExpression | { kind: 'ObjectExpression'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 9d41663d56..02994ef0f9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -2115,10 +2115,17 @@ function codegenInstructionValue( } case 'TypeCastExpression': { if (t.isTSType(instrValue.typeAnnotation)) { - value = t.tsAsExpression( - codegenPlaceToExpression(cx, instrValue.value), - instrValue.typeAnnotation, - ); + if (instrValue.typeAnnotationKind === 'satisfies') { + value = t.tsSatisfiesExpression( + codegenPlaceToExpression(cx, instrValue.value), + instrValue.typeAnnotation, + ); + } else { + value = t.tsAsExpression( + codegenPlaceToExpression(cx, instrValue.value), + instrValue.typeAnnotation, + ); + } } else { value = t.typeCastExpression( codegenPlaceToExpression(cx, instrValue.value), diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-array.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-array.expect.md new file mode 100644 index 0000000000..d0083f3c3e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-array.expect.md @@ -0,0 +1,71 @@ + +## Input + +```javascript +// @enableUseTypeAnnotations +function Component(props: {id: number}) { + const x = makeArray(props.id) satisfies number[]; + const y = x.at(0); + return y; +} + +function makeArray(x: T): Array { + return [x]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{id: 42}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableUseTypeAnnotations +function Component(props) { + const $ = _c(4); + let t0; + if ($[0] !== props.id) { + t0 = makeArray(props.id); + $[0] = props.id; + $[1] = t0; + } else { + t0 = $[1]; + } + const x = t0 satisfies number[]; + let t1; + if ($[2] !== x) { + t1 = x.at(0); + $[2] = x; + $[3] = t1; + } else { + t1 = $[3]; + } + const y = t1; + return y; +} + +function makeArray(x) { + const $ = _c(2); + let t0; + if ($[0] !== x) { + t0 = [x]; + $[0] = x; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ id: 42 }], +}; + +``` + +### Eval output +(kind: ok) 42 \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-array.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-array.ts new file mode 100644 index 0000000000..5024055c10 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-array.ts @@ -0,0 +1,15 @@ +// @enableUseTypeAnnotations +function Component(props: {id: number}) { + const x = makeArray(props.id) satisfies number[]; + const y = x.at(0); + return y; +} + +function makeArray(x: T): Array { + return [x]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{id: 42}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-number.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-number.expect.md new file mode 100644 index 0000000000..0819456dc1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-number.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +// @enableUseTypeAnnotations +import {identity} from 'shared-runtime'; + +function Component(props: {id: number}) { + const x = identity(props.id); + const y = x satisfies number; + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{id: 42}], +}; + +``` + +## Code + +```javascript +// @enableUseTypeAnnotations +import { identity } from "shared-runtime"; + +function Component(props) { + const x = identity(props.id); + const y = x satisfies number; + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ id: 42 }], +}; + +``` + +### Eval output +(kind: ok) 42 \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-number.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-number.ts new file mode 100644 index 0000000000..2f4ea6222e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-annotations/type-annotation-satisfies-number.ts @@ -0,0 +1,13 @@ +// @enableUseTypeAnnotations +import {identity} from 'shared-runtime'; + +function Component(props: {id: number}) { + const x = identity(props.id); + const y = x satisfies number; + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{id: 42}], +}; From b2b0b8bb5a09e11de9d8489047b53ad186f8e807 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 28 Mar 2025 13:59:41 -0400 Subject: [PATCH 12/13] [release] Also split the onlyPackages param I missed this the last time. --- .../release/prepare-release-from-npm-commands/parse-params.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/release/prepare-release-from-npm-commands/parse-params.js b/scripts/release/prepare-release-from-npm-commands/parse-params.js index ef9c4979b2..10dbfb4e51 100644 --- a/scripts/release/prepare-release-from-npm-commands/parse-params.js +++ b/scripts/release/prepare-release-from-npm-commands/parse-params.js @@ -56,6 +56,7 @@ module.exports = () => { const params = commandLineArgs(paramDefinitions); splitCommaParams(params.skipPackages); + splitCommaParams(params.onlyPackages); return params; }; From df10e773467b5e3b091e071cfbd260471dafd598 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 28 Mar 2025 14:05:42 -0400 Subject: [PATCH 13/13] [release] Don't lookup build-info.json when updating version numbers From what we can see, `build-info.json` is a vestigal file that we were previously including in builds but are no longer since 2022 (see https://github.com/facebook/react/pull/23257, which removes `build-info.json` which would have broken scripts/release/build-release-locally-commands/add-build-info-json.js). Since this file is no longer built, instead of looking it up we default to the `version` that was passed in as an argument to scripts/release/prepare-release-from-npm.js. Since `version` is what is pulled from npm, there should only be 1 consistent version for all the packages that are pulled. Therefore, only 1 version (eg canary) needs to be replaced to the new stable version. --- .../update-stable-version-numbers.js | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/scripts/release/prepare-release-from-npm-commands/update-stable-version-numbers.js b/scripts/release/prepare-release-from-npm-commands/update-stable-version-numbers.js index f201118890..b288c0ab5f 100644 --- a/scripts/release/prepare-release-from-npm-commands/update-stable-version-numbers.js +++ b/scripts/release/prepare-release-from-npm-commands/update-stable-version-numbers.js @@ -117,16 +117,7 @@ const run = async ({cwd, packages, version, ci}, versionsMap) => { // A separate "React version" is used for the embedded renderer version to support DevTools, // since it needs to distinguish between different version ranges of React. // We need to replace it as well as the "next" version number. - const buildInfoPath = join(nodeModulesPath, 'react', 'build-info.json'); - const {reactVersion} = await readJson(buildInfoPath); - - if (!reactVersion) { - console.error( - theme`{error Unsupported or invalid build metadata in} {path build/node_modules/react/build-info.json}` + - theme`{error . This could indicate that you have specified an outdated "next" version.}` - ); - process.exit(1); - } + const reactVersion = version; // We print the diff to the console for review, // but it can be large so let's also write it to disk. @@ -152,10 +143,6 @@ const run = async ({cwd, packages, version, ci}, versionsMap) => { while (afterContents.indexOf(version) >= 0) { afterContents = afterContents.replace(version, newStableVersion); } - // Replace inline renderer version numbers (e.g. shared/ReactVersion). - while (afterContents.indexOf(reactVersion) >= 0) { - afterContents = afterContents.replace(reactVersion, newStableVersion); - } if (beforeContents !== afterContents) { numFilesModified++; // Using a relative path for diff helps with the snapshot test