diff --git a/.github/workflows/compiler_discord_notify.yml b/.github/workflows/compiler_discord_notify.yml index 7a5f5db0fb988..5a57cf6a32c19 100644 --- a/.github/workflows/compiler_discord_notify.yml +++ b/.github/workflows/compiler_discord_notify.yml @@ -11,6 +11,7 @@ permissions: {} jobs: check_access: + if: ${{ github.event.pull_request.draft == false }} runs-on: ubuntu-latest outputs: is_member_or_collaborator: ${{ steps.check_is_member_or_collaborator.outputs.is_member_or_collaborator }} diff --git a/.github/workflows/runtime_build_and_test.yml b/.github/workflows/runtime_build_and_test.yml index 1d0a896984e26..0d87776d0494a 100644 --- a/.github/workflows/runtime_build_and_test.yml +++ b/.github/workflows/runtime_build_and_test.yml @@ -280,6 +280,35 @@ jobs: if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn test ${{ matrix.params }} --ci --shard=${{ matrix.shard }} + # Hardcoded to improve parallelism + test-linter: + name: Test eslint-plugin-react-hooks + needs: [runtime_compiler_node_modules_cache] + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + cache: yarn + cache-dependency-path: | + yarn.lock + compiler/yarn.lock + - name: Restore cached node_modules + uses: actions/cache@v4 + id: node_modules + with: + path: | + **/node_modules + key: runtime-and-compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('yarn.lock', 'compiler/yarn.lock') }} + restore-keys: | + runtime-and-compiler-node_modules-v6-${{ runner.arch }}-${{ runner.os }}- + runtime-and-compiler-node_modules-v6- + - run: yarn install --frozen-lockfile + if: steps.node_modules.outputs.cache-hit != 'true' + - run: ./scripts/react-compiler/build-compiler.sh && ./scripts/react-compiler/link-compiler.sh + - run: yarn workspace eslint-plugin-react-hooks test + # ----- BUILD ----- build_and_lint: name: yarn build and lint diff --git a/.github/workflows/runtime_discord_notify.yml b/.github/workflows/runtime_discord_notify.yml index 69e4c3453f343..8d047e697640d 100644 --- a/.github/workflows/runtime_discord_notify.yml +++ b/.github/workflows/runtime_discord_notify.yml @@ -11,6 +11,7 @@ permissions: {} jobs: check_access: + if: ${{ github.event.pull_request.draft == false }} runs-on: ubuntu-latest outputs: is_member_or_collaborator: ${{ steps.check_is_member_or_collaborator.outputs.is_member_or_collaborator }} diff --git a/.github/workflows/shared_stale.yml b/.github/workflows/shared_stale.yml index a2c707973c927..c24895edc5da7 100644 --- a/.github/workflows/shared_stale.yml +++ b/.github/workflows/shared_stale.yml @@ -6,7 +6,10 @@ on: - cron: '0 * * * *' workflow_dispatch: -permissions: {} +permissions: + # https://github.com/actions/stale/tree/v9/?tab=readme-ov-file#recommended-permissions + issues: write + pull-requests: write env: TZ: /usr/share/zoneinfo/America/Los_Angeles diff --git a/compiler/apps/playground/scripts/link-compiler.sh b/compiler/apps/playground/scripts/link-compiler.sh index 1ee5f0b81bf09..96188f7b45137 100755 --- a/compiler/apps/playground/scripts/link-compiler.sh +++ b/compiler/apps/playground/scripts/link-compiler.sh @@ -8,8 +8,8 @@ set -eo pipefail HERE=$(pwd) -cd ../../packages/react-compiler-runtime && yarn --silent link && cd $HERE -cd ../../packages/babel-plugin-react-compiler && yarn --silent link && cd $HERE +cd ../../packages/react-compiler-runtime && yarn --silent link && cd "$HERE" +cd ../../packages/babel-plugin-react-compiler && yarn --silent link && cd "$HERE" yarn --silent link babel-plugin-react-compiler yarn --silent link react-compiler-runtime diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index b8504494662d6..cc11d0faceb18 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -17,6 +17,7 @@ import { BuiltInSetId, BuiltInUseActionStateId, BuiltInUseContextHookId, + BuiltInUseEffectEventId, BuiltInUseEffectHookId, BuiltInUseInsertionEffectHookId, BuiltInUseLayoutEffectHookId, @@ -27,6 +28,7 @@ import { BuiltInUseTransitionId, BuiltInWeakMapId, BuiltInWeakSetId, + BuiltinEffectEventId, ReanimatedSharedValueId, ShapeRegistry, addFunction, @@ -722,6 +724,27 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ BuiltInFireId, ), ], + [ + 'useEffectEvent', + addHook( + DEFAULT_SHAPES, + { + positionalParams: [], + restParam: Effect.Freeze, + returnType: { + kind: 'Function', + return: {kind: 'Poly'}, + shapeId: BuiltinEffectEventId, + isConstructor: false, + }, + calleeEffect: Effect.Read, + hookKind: 'useEffectEvent', + // Frozen because it should not mutate any locally-bound values + returnValueKind: ValueKind.Frozen, + }, + BuiltInUseEffectEventId, + ), + ], ]; TYPED_GLOBALS.push( diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 1699a0fc3d292..6c55ff22bc649 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1785,6 +1785,13 @@ export function isFireFunctionType(id: Identifier): boolean { ); } +export function isEffectEventFunctionType(id: Identifier): boolean { + return ( + id.type.kind === 'Function' && + id.type.shapeId === 'BuiltInEffectEventFunction' + ); +} + export function isStableType(id: Identifier): boolean { return ( isSetStateType(id) || diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index 03f4120149b0e..a017e1479a22b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -131,6 +131,7 @@ export type HookKind = | 'useCallback' | 'useTransition' | 'useImperativeHandle' + | 'useEffectEvent' | 'Custom'; /* @@ -226,6 +227,8 @@ export const BuiltInUseTransitionId = 'BuiltInUseTransition'; export const BuiltInStartTransitionId = 'BuiltInStartTransition'; export const BuiltInFireId = 'BuiltInFire'; export const BuiltInFireFunctionId = 'BuiltInFireFunction'; +export const BuiltInUseEffectEventId = 'BuiltInUseEffectEvent'; +export const BuiltinEffectEventId = 'BuiltInEffectEventFunction'; // See getReanimatedModuleType() in Globals.ts — this is part of supporting Reanimated's ref-like types export const ReanimatedSharedValueId = 'ReanimatedSharedValueId'; @@ -948,6 +951,19 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, [ ['*', {kind: 'Object', shapeId: BuiltInRefValueId}], ]); +addFunction( + BUILTIN_SHAPES, + [], + { + positionalParams: [], + restParam: Effect.ConditionallyMutate, + returnType: {kind: 'Poly'}, + calleeEffect: Effect.ConditionallyMutate, + returnValueKind: ValueKind.Mutable, + }, + BuiltinEffectEventId, +); + /** * MixedReadOnly = * | primitive 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 4daa2f9fbaee7..eab3c241bcccf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -31,6 +31,7 @@ import { HIR, BasicBlock, BlockId, + isEffectEventFunctionType, } from '../HIR'; import {collectHoistablePropertyLoadsInInnerFn} from '../HIR/CollectHoistablePropertyLoads'; import {collectOptionalChainSidemap} from '../HIR/CollectOptionalChainDependencies'; @@ -209,7 +210,8 @@ export function inferEffectDependencies(fn: HIRFunction): void { ((isUseRefType(maybeDep.identifier) || isSetStateType(maybeDep.identifier)) && !reactiveIds.has(maybeDep.identifier.id)) || - isFireFunctionType(maybeDep.identifier) + isFireFunctionType(maybeDep.identifier) || + isEffectEventFunctionType(maybeDep.identifier) ) { // exclude non-reactive hook results, which will never be in a memo block continue; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-ref-prefix-postfix-operator.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-ref-prefix-postfix-operator.expect.md new file mode 100644 index 0000000000000..ccfc451750004 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-ref-prefix-postfix-operator.expect.md @@ -0,0 +1,132 @@ + +## Input + +```javascript +import {useRef, useEffect} from 'react'; + +/** + * The postfix increment operator should return the value before incrementing. + * ```js + * const id = count.current; // 0 + * count.current = count.current + 1; // 1 + * return id; + * ``` + * The bug is that we currently increment the value before the expression is evaluated. + * This bug does not trigger when the incremented value is a plain primitive. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"} + * logs: ['id = 0','count = 1'] + * Forget: + * (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"} + * logs: ['id = 1','count = 1'] + */ +function useFoo() { + const count = useRef(0); + const updateCountPostfix = () => { + const id = count.current++; + return id; + }; + const updateCountPrefix = () => { + const id = ++count.current; + return id; + }; + useEffect(() => { + const id = updateCountPostfix(); + console.log(`id = ${id}`); + console.log(`count = ${count.current}`); + }, []); + return {count, updateCountPostfix, updateCountPrefix}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useRef, useEffect } from "react"; + +/** + * The postfix increment operator should return the value before incrementing. + * ```js + * const id = count.current; // 0 + * count.current = count.current + 1; // 1 + * return id; + * ``` + * The bug is that we currently increment the value before the expression is evaluated. + * This bug does not trigger when the incremented value is a plain primitive. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"} + * logs: ['id = 0','count = 1'] + * Forget: + * (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"} + * logs: ['id = 1','count = 1'] + */ +function useFoo() { + const $ = _c(5); + const count = useRef(0); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = () => { + count.current = count.current + 1; + const id = count.current; + return id; + }; + $[0] = t0; + } else { + t0 = $[0]; + } + const updateCountPostfix = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = () => { + const id_0 = (count.current = count.current + 1); + return id_0; + }; + $[1] = t1; + } else { + t1 = $[1]; + } + const updateCountPrefix = t1; + let t2; + let t3; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t2 = () => { + const id_1 = updateCountPostfix(); + console.log(`id = ${id_1}`); + console.log(`count = ${count.current}`); + }; + t3 = []; + $[2] = t2; + $[3] = t3; + } else { + t2 = $[2]; + t3 = $[3]; + } + useEffect(t2, t3); + let t4; + if ($[4] === Symbol.for("react.memo_cache_sentinel")) { + t4 = { count, updateCountPostfix, updateCountPrefix }; + $[4] = t4; + } else { + t4 = $[4]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-ref-prefix-postfix-operator.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-ref-prefix-postfix-operator.js new file mode 100644 index 0000000000000..a7c1fad8bf231 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-ref-prefix-postfix-operator.js @@ -0,0 +1,42 @@ +import {useRef, useEffect} from 'react'; + +/** + * The postfix increment operator should return the value before incrementing. + * ```js + * const id = count.current; // 0 + * count.current = count.current + 1; // 1 + * return id; + * ``` + * The bug is that we currently increment the value before the expression is evaluated. + * This bug does not trigger when the incremented value is a plain primitive. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"} + * logs: ['id = 0','count = 1'] + * Forget: + * (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"} + * logs: ['id = 1','count = 1'] + */ +function useFoo() { + const count = useRef(0); + const updateCountPostfix = () => { + const id = count.current++; + return id; + }; + const updateCountPrefix = () => { + const id = ++count.current; + return id; + }; + useEffect(() => { + const id = updateCountPostfix(); + console.log(`id = ${id}`); + console.log(`count = ${count.current}`); + }, []); + return {count, updateCountPostfix, updateCountPrefix}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/nonreactive-effect-event.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/nonreactive-effect-event.expect.md new file mode 100644 index 0000000000000..56c5f6f6f8807 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/nonreactive-effect-event.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect, useEffectEvent} from 'react'; +import {print} from 'shared-runtime'; + +/** + * We do not include effect events in dep arrays. + */ +function NonReactiveEffectEvent() { + const fn = useEffectEvent(() => print('hello world')); + useEffect(() => fn()); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect, useEffectEvent } from "react"; +import { print } from "shared-runtime"; + +/** + * We do not include effect events in dep arrays. + */ +function NonReactiveEffectEvent() { + const $ = _c(2); + const fn = useEffectEvent(_temp); + let t0; + if ($[0] !== fn) { + t0 = () => fn(); + $[0] = fn; + $[1] = t0; + } else { + t0 = $[1]; + } + useEffect(t0, []); +} +function _temp() { + return print("hello world"); +} + +``` + +### 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/nonreactive-effect-event.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/nonreactive-effect-event.js new file mode 100644 index 0000000000000..02706c6b23735 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/nonreactive-effect-event.js @@ -0,0 +1,11 @@ +// @inferEffectDependencies +import {useEffect, useEffectEvent} from 'react'; +import {print} from 'shared-runtime'; + +/** + * We do not include effect events in dep arrays. + */ +function NonReactiveEffectEvent() { + const fn = useEffectEvent(() => print('hello world')); + useEffect(() => fn()); +} diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 62b8a7703fddb..d7c202956178c 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -460,6 +460,7 @@ const skipFilter = new Set([ 'fbt/bug-fbt-plural-multiple-function-calls', 'fbt/bug-fbt-plural-multiple-mixed-call-tag', 'bug-invalid-phi-as-dependency', + 'bug-ref-prefix-postfix-operator', // 'react-compiler-runtime' not yet supported 'flag-enable-emit-hook-guards', diff --git a/fixtures/flight/server/global.js b/fixtures/flight/server/global.js index a2fa737ae0f4d..d81dc2c038c12 100644 --- a/fixtures/flight/server/global.js +++ b/fixtures/flight/server/global.js @@ -101,6 +101,9 @@ async function renderApp(req, res, next) { } else if (req.get('Content-type')) { proxiedHeaders['Content-type'] = req.get('Content-type'); } + if (req.headers['cache-control']) { + proxiedHeaders['Cache-Control'] = req.get('cache-control'); + } const requestsPrerender = req.path === '/prerender'; diff --git a/fixtures/flight/server/region.js b/fixtures/flight/server/region.js index 6896713e41cbf..a352d34ee6b79 100644 --- a/fixtures/flight/server/region.js +++ b/fixtures/flight/server/region.js @@ -50,7 +50,7 @@ const {readFile} = require('fs').promises; const React = require('react'); -async function renderApp(res, returnValue, formState) { +async function renderApp(res, returnValue, formState, noCache) { const {renderToPipeableStream} = await import( 'react-server-dom-webpack/server' ); @@ -97,7 +97,7 @@ async function renderApp(res, returnValue, formState) { key: filename, }) ), - React.createElement(App) + React.createElement(App, {noCache}) ); // For client-invoked server actions we refresh the tree and return a return value. const payload = {root, returnValue, formState}; @@ -105,7 +105,7 @@ async function renderApp(res, returnValue, formState) { pipe(res); } -async function prerenderApp(res, returnValue, formState) { +async function prerenderApp(res, returnValue, formState, noCache) { const {unstable_prerenderToNodeStream: prerenderToNodeStream} = await import( 'react-server-dom-webpack/static' ); @@ -152,7 +152,7 @@ async function prerenderApp(res, returnValue, formState) { key: filename, }) ), - React.createElement(App, {prerender: true}) + React.createElement(App, {prerender: true, noCache}) ); // For client-invoked server actions we refresh the tree and return a return value. const payload = {root, returnValue, formState}; @@ -161,14 +161,17 @@ async function prerenderApp(res, returnValue, formState) { } app.get('/', async function (req, res) { + const noCache = req.get('cache-control') === 'no-cache'; + if ('prerender' in req.query) { - await prerenderApp(res, null, null); + await prerenderApp(res, null, null, noCache); } else { - await renderApp(res, null, null); + await renderApp(res, null, null, noCache); } }); app.post('/', bodyParser.text(), async function (req, res) { + const noCache = req.headers['cache-control'] === 'no-cache'; const {decodeReply, decodeReplyFromBusboy, decodeAction, decodeFormState} = await import('react-server-dom-webpack/server'); const serverReference = req.get('rsc-action'); @@ -201,7 +204,7 @@ app.post('/', bodyParser.text(), async function (req, res) { // We handle the error on the client } // Refresh the client and return the value - renderApp(res, result, null); + renderApp(res, result, null, noCache); } else { // This is the progressive enhancement case const UndiciRequest = require('undici').Request; @@ -217,11 +220,11 @@ app.post('/', bodyParser.text(), async function (req, res) { // Wait for any mutations const result = await action(); const formState = decodeFormState(result, formData); - renderApp(res, null, formState); + renderApp(res, null, formState, noCache); } catch (x) { const {setServerState} = await import('../src/ServerState.js'); setServerState('Error: ' + x.message); - renderApp(res, null, null); + renderApp(res, null, null, noCache); } } }); diff --git a/fixtures/flight/src/App.js b/fixtures/flight/src/App.js index 08eaefc90f887..2f29de7abaffe 100644 --- a/fixtures/flight/src/App.js +++ b/fixtures/flight/src/App.js @@ -1,6 +1,6 @@ import * as React from 'react'; -import {renderToPipeableStream} from 'react-server-dom-webpack/server'; -import {createFromNodeStream} from 'react-server-dom-webpack/client'; +import {renderToReadableStream} from 'react-server-dom-webpack/server'; +import {createFromReadableStream} from 'react-server-dom-webpack/client'; import {PassThrough, Readable} from 'stream'; import Container from './Container.js'; @@ -33,62 +33,88 @@ function Foo({children}) { return
{children}
; } +async function delay(text, ms) { + return new Promise(resolve => setTimeout(() => resolve(text), ms)); +} + +async function delayTwice() { + await delay('', 20); + await delay('', 10); +} + +async function delayTrice() { + const p = delayTwice(); + await delay('', 40); + return p; +} + async function Bar({children}) { - await new Promise(resolve => setTimeout(() => resolve('deferred text'), 10)); + await delayTrice(); return
{children}
; } async function ThirdPartyComponent() { - return new Promise(resolve => - setTimeout(() => resolve('hello from a 3rd party'), 30) - ); + return await delay('hello from a 3rd party', 30); } -// Using Web streams for tee'ing convenience here. -let cachedThirdPartyReadableWeb; - -function fetchThirdParty(Component) { - if (cachedThirdPartyReadableWeb) { - const [readableWeb1, readableWeb2] = cachedThirdPartyReadableWeb.tee(); - cachedThirdPartyReadableWeb = readableWeb1; +let cachedThirdPartyStream; + +// We create the Component outside of AsyncLocalStorage so that it has no owner. +// That way it gets the owner from the call to createFromNodeStream. +const thirdPartyComponent = ; + +function simulateFetch(cb, latencyMs) { + return new Promise(resolve => { + // Request latency + setTimeout(() => { + const result = cb(); + // Response latency + setTimeout(() => { + resolve(result); + }, latencyMs); + }, latencyMs); + }); +} - return createFromNodeStream(Readable.fromWeb(readableWeb2), { - moduleMap: {}, - moduleLoading: {}, - }); +async function fetchThirdParty(noCache) { + // We're using the Web Streams APIs for tee'ing convenience. + let stream; + if (cachedThirdPartyStream && !noCache) { + stream = cachedThirdPartyStream; + } else { + stream = await simulateFetch( + () => + renderToReadableStream( + thirdPartyComponent, + {}, + {environmentName: 'third-party'} + ), + 25 + ); } - const stream = renderToPipeableStream( - , - {}, - {environmentName: 'third-party'} - ); + const [stream1, stream2] = stream.tee(); + cachedThirdPartyStream = stream1; - const readable = new PassThrough(); - // React currently only supports piping to one stream, so we convert, tee, and - // convert back again. - // TODO: Switch to web streams without converting when #33442 has landed. - const [readableWeb1, readableWeb2] = Readable.toWeb(readable).tee(); - cachedThirdPartyReadableWeb = readableWeb1; - const result = createFromNodeStream(Readable.fromWeb(readableWeb2), { - moduleMap: {}, - moduleLoading: {}, + return createFromReadableStream(stream2, { + serverConsumerManifest: { + moduleMap: {}, + serverModuleMap: null, + moduleLoading: null, + }, }); - stream.pipe(readable); - - return result; } -async function ServerComponent() { - await new Promise(resolve => setTimeout(() => resolve('deferred text'), 50)); - return await fetchThirdParty(); +async function ServerComponent({noCache}) { + await delay('deferred text', 50); + return await fetchThirdParty(noCache); } -export default async function App({prerender}) { +export default async function App({prerender, noCache}) { const res = await fetch('http://localhost:3001/todos'); const todos = await res.json(); - const dedupedChild = ; + const dedupedChild = ; const message = getServerState(); return ( diff --git a/fixtures/view-transition/src/components/App.js b/fixtures/view-transition/src/components/App.js index 275e594d87a1d..dd8dcb73a2ef2 100644 --- a/fixtures/view-transition/src/components/App.js +++ b/fixtures/view-transition/src/components/App.js @@ -4,6 +4,7 @@ import React, { useEffect, useState, unstable_addTransitionType as addTransitionType, + use, } from 'react'; import Chrome from './Chrome'; diff --git a/fixtures/view-transition/src/components/NestedReveal.js b/fixtures/view-transition/src/components/NestedReveal.js new file mode 100644 index 0000000000000..497f4430f6cfb --- /dev/null +++ b/fixtures/view-transition/src/components/NestedReveal.js @@ -0,0 +1,36 @@ +import React, {Suspense, use} from 'react'; + +async function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +function Use({useable}) { + use(useable); + return null; +} + +let delay1; +let delay2; + +export default function NestedReveal({}) { + if (!delay1) { + delay1 = sleep(100); + // Needs to happen before the throttled reveal of delay 1 + delay2 = sleep(200); + } + + return ( +
+ Shell + +
Level 1
+ + + +
Level 2
+ + + +
+ ); +} diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 39d0803af7700..c0d6f7a0a24ca 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -18,6 +18,7 @@ import SwipeRecognizer from './SwipeRecognizer'; import './Page.css'; import transitions from './Transitions.module.css'; +import NestedReveal from './NestedReveal'; async function sleep(ms) { return new Promise(resolve => setTimeout(resolve, ms)); @@ -241,6 +242,7 @@ export default function Page({url, navigate}) { + ); } diff --git a/package.json b/package.json index 6ad9211568541..9e7eb8dc5334d 100644 --- a/package.json +++ b/package.json @@ -129,7 +129,6 @@ "lint-build": "node ./scripts/rollup/validate/index.js", "extract-errors": "node scripts/error-codes/extract-errors.js", "postinstall": "node ./scripts/flow/createFlowConfigs.js", - "pretest": "./scripts/react-compiler/build-compiler.sh && ./scripts/react-compiler/link-compiler.sh", "test": "node ./scripts/jest/jest-cli.js", "test-stable": "node ./scripts/jest/jest-cli.js --release-channel=stable", "test-www": "node ./scripts/jest/jest-cli.js --release-channel=www-modern", diff --git a/packages/internal-test-utils/consoleMock.js b/packages/internal-test-utils/consoleMock.js index 9bb797bac395b..ecb97b3a03059 100644 --- a/packages/internal-test-utils/consoleMock.js +++ b/packages/internal-test-utils/consoleMock.js @@ -156,7 +156,8 @@ function normalizeCodeLocInfo(str) { // at Component (/path/filename.js:123:45) // React format: // in Component (at filename.js:123) - return str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) { + return str.replace(/\n +(?:at|in) ([^(\[\n]+)[^\n]*/g, function (m, name) { + name = name.trim(); if (name.endsWith('.render')) { // Class components will have the `render` method as part of their stack trace. // We strip that out in our normalization to make it look more like component stacks. diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 78a2d85eea287..1171f933d2712 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -844,7 +844,7 @@ function createElement( // This owner should ideally have already been initialized to avoid getting // user stack frames on the stack. const ownerTask = - owner === null ? null : initializeFakeTask(response, owner, env); + owner === null ? null : initializeFakeTask(response, owner); if (ownerTask === null) { const rootTask = response._debugRootTask; if (rootTask != null) { @@ -2494,7 +2494,6 @@ function getRootTask( function initializeFakeTask( response: Response, debugInfo: ReactComponentInfo | ReactAsyncInfo | ReactIOInfo, - childEnvironmentName: string, ): null | ConsoleTask { if (!supportsCreateTask) { return null; @@ -2504,6 +2503,10 @@ function initializeFakeTask( // If it's null, we can't initialize a task. return null; } + const cachedEntry = debugInfo.debugTask; + if (cachedEntry !== undefined) { + return cachedEntry; + } // Workaround for a bug where Chrome Performance tracking uses the enclosing line/column // instead of the callsite. For ReactAsyncInfo/ReactIOInfo, the only thing we're going @@ -2516,47 +2519,35 @@ function initializeFakeTask( const stack = debugInfo.stack; const env: string = debugInfo.env == null ? response._rootEnvironmentName : debugInfo.env; - if (env !== childEnvironmentName) { + const ownerEnv: string = + debugInfo.owner == null || debugInfo.owner.env == null + ? response._rootEnvironmentName + : debugInfo.owner.env; + const ownerTask = + debugInfo.owner == null + ? null + : initializeFakeTask(response, debugInfo.owner); + const taskName = // This is the boundary between two environments so we'll annotate the task name. - // That is unusual so we don't cache it. - const ownerTask = - debugInfo.owner == null - ? null - : initializeFakeTask(response, debugInfo.owner, env); - return buildFakeTask( - response, - ownerTask, - stack, - '"use ' + childEnvironmentName.toLowerCase() + '"', - env, - useEnclosingLine, - ); - } else { - const cachedEntry = debugInfo.debugTask; - if (cachedEntry !== undefined) { - return cachedEntry; - } - const ownerTask = - debugInfo.owner == null - ? null - : initializeFakeTask(response, debugInfo.owner, env); - // Some unfortunate pattern matching to refine the type. - const taskName = - debugInfo.key !== undefined + // We assume that the stack frame of the entry into the new environment was done + // from the old environment. So we use the owner's environment as the current. + env !== ownerEnv + ? '"use ' + env.toLowerCase() + '"' + : // Some unfortunate pattern matching to refine the type. + debugInfo.key !== undefined ? getServerComponentTaskName(((debugInfo: any): ReactComponentInfo)) : debugInfo.name !== undefined ? getIOInfoTaskName(((debugInfo: any): ReactIOInfo)) : getAsyncInfoTaskName(((debugInfo: any): ReactAsyncInfo)); - // $FlowFixMe[cannot-write]: We consider this part of initialization. - return (debugInfo.debugTask = buildFakeTask( - response, - ownerTask, - stack, - taskName, - env, - useEnclosingLine, - )); - } + // $FlowFixMe[cannot-write]: We consider this part of initialization. + return (debugInfo.debugTask = buildFakeTask( + response, + ownerTask, + stack, + taskName, + ownerEnv, + useEnclosingLine, + )); } function buildFakeTask( @@ -2658,27 +2649,30 @@ function resolveDebugInfo( 'resolveDebugInfo should never be called in production mode. This is a bug in React.', ); } - // We eagerly initialize the fake task because this resolving happens outside any - // render phase so we're not inside a user space stack at this point. If we waited - // to initialize it when we need it, we might be inside user code. - const env = - debugInfo.env === undefined ? response._rootEnvironmentName : debugInfo.env; if (debugInfo.stack !== undefined) { const componentInfoOrAsyncInfo: ReactComponentInfo | ReactAsyncInfo = // $FlowFixMe[incompatible-type] debugInfo; - initializeFakeTask(response, componentInfoOrAsyncInfo, env); + // We eagerly initialize the fake task because this resolving happens outside any + // render phase so we're not inside a user space stack at this point. If we waited + // to initialize it when we need it, we might be inside user code. + initializeFakeTask(response, componentInfoOrAsyncInfo); } - if (debugInfo.owner === null && response._debugRootOwner != null) { + if (debugInfo.owner == null && response._debugRootOwner != null) { const componentInfoOrAsyncInfo: ReactComponentInfo | ReactAsyncInfo = // $FlowFixMe: By narrowing `owner` to `null`, we narrowed `debugInfo` to `ReactComponentInfo` debugInfo; // $FlowFixMe[cannot-write] componentInfoOrAsyncInfo.owner = response._debugRootOwner; + // We clear the parsed stack frames to indicate that it needs to be re-parsed from debugStack. + // $FlowFixMe[cannot-write] + componentInfoOrAsyncInfo.stack = null; // We override the stack if we override the owner since the stack where the root JSX // was created on the server isn't very useful but where the request was made is. // $FlowFixMe[cannot-write] componentInfoOrAsyncInfo.debugStack = response._debugRootStack; + // $FlowFixMe[cannot-write] + componentInfoOrAsyncInfo.debugTask = response._debugRootTask; } else if (debugInfo.stack !== undefined) { const componentInfoOrAsyncInfo: ReactComponentInfo | ReactAsyncInfo = // $FlowFixMe[incompatible-type] @@ -2738,7 +2732,7 @@ const replayConsoleWithCallStack = { bindToConsole(methodName, args, env), ); if (owner != null) { - const task = initializeFakeTask(response, owner, env); + const task = initializeFakeTask(response, owner); initializeFakeStack(response, owner); if (task !== null) { task.run(callStack); @@ -2812,10 +2806,8 @@ function resolveConsoleEntry( } function initializeIOInfo(response: Response, ioInfo: ReactIOInfo): void { - const env = - ioInfo.env === undefined ? response._rootEnvironmentName : ioInfo.env; if (ioInfo.stack !== undefined) { - initializeFakeTask(response, ioInfo, env); + initializeFakeTask(response, ioInfo); initializeFakeStack(response, ioInfo); } // Adjust the time to the current environment's time space. @@ -2910,6 +2902,46 @@ function resolveTypedArray( resolveBuffer(response, id, view); } +function logComponentInfo( + response: Response, + root: SomeChunk, + componentInfo: ReactComponentInfo, + trackIdx: number, + startTime: number, + componentEndTime: number, + childrenEndTime: number, + isLastComponent: boolean, +): void { + // $FlowFixMe: Refined. + if ( + isLastComponent && + root.status === ERRORED && + root.reason !== response._closedReason + ) { + // If this is the last component to render before this chunk rejected, then conceptually + // this component errored. If this was a cancellation then it wasn't this component that + // errored. + logComponentErrored( + componentInfo, + trackIdx, + startTime, + componentEndTime, + childrenEndTime, + response._rootEnvironmentName, + root.reason, + ); + } else { + logComponentRender( + componentInfo, + trackIdx, + startTime, + componentEndTime, + childrenEndTime, + response._rootEnvironmentName, + ); + } +} + function flushComponentPerformance( response: Response, root: SomeChunk, @@ -2965,21 +2997,20 @@ function flushComponentPerformance( // in parallel with the previous. const debugInfo = __DEV__ && root._debugInfo; if (debugInfo) { - for (let i = 1; i < debugInfo.length; i++) { + let startTime = 0; + for (let i = 0; i < debugInfo.length; i++) { const info = debugInfo[i]; + if (typeof info.time === 'number') { + startTime = info.time; + } if (typeof info.name === 'string') { - // $FlowFixMe: Refined. - const startTimeInfo = debugInfo[i - 1]; - if (typeof startTimeInfo.time === 'number') { - const startTime = startTimeInfo.time; - if (startTime < trackTime) { - // The start time of this component is before the end time of the previous - // component on this track so we need to bump the next one to a parallel track. - trackIdx++; - } - trackTime = startTime; - break; + if (startTime < trackTime) { + // The start time of this component is before the end time of the previous + // component on this track so we need to bump the next one to a parallel track. + trackIdx++; } + trackTime = startTime; + break; } } for (let i = debugInfo.length - 1; i >= 0; i--) { @@ -2987,6 +3018,7 @@ function flushComponentPerformance( if (typeof info.time === 'number') { if (info.time > parentEndTime) { parentEndTime = info.time; + break; // We assume the highest number is at the end. } } } @@ -3014,85 +3046,72 @@ function flushComponentPerformance( } childTrackIdx = childResult.track; const childEndTime = childResult.endTime; - childTrackTime = childEndTime; + if (childEndTime > childTrackTime) { + childTrackTime = childEndTime; + } if (childEndTime > childrenEndTime) { childrenEndTime = childEndTime; } } if (debugInfo) { - let endTime = 0; + // Write debug info in reverse order (just like stack traces). + let componentEndTime = 0; let isLastComponent = true; + let endTime = -1; + let endTimeIdx = -1; for (let i = debugInfo.length - 1; i >= 0; i--) { const info = debugInfo[i]; - if (typeof info.time === 'number') { - if (info.time > childrenEndTime) { - childrenEndTime = info.time; - } - if (endTime === 0) { - // Last timestamp is the end of the last component. - endTime = info.time; - } + if (typeof info.time !== 'number') { + continue; } - if (typeof info.name === 'string' && i > 0) { - // $FlowFixMe: Refined. - const componentInfo: ReactComponentInfo = info; - const startTimeInfo = debugInfo[i - 1]; - if (typeof startTimeInfo.time === 'number') { - const startTime = startTimeInfo.time; - if ( - isLastComponent && - root.status === ERRORED && - root.reason !== response._closedReason - ) { - // If this is the last component to render before this chunk rejected, then conceptually - // this component errored. If this was a cancellation then it wasn't this component that - // errored. - logComponentErrored( + if (componentEndTime === 0) { + // Last timestamp is the end of the last component. + componentEndTime = info.time; + } + const time = info.time; + if (endTimeIdx > -1) { + // Now that we know the start and end time, we can emit the entries between. + for (let j = endTimeIdx - 1; j > i; j--) { + const candidateInfo = debugInfo[j]; + if (typeof candidateInfo.name === 'string') { + if (componentEndTime > childrenEndTime) { + childrenEndTime = componentEndTime; + } + // $FlowFixMe: Refined. + const componentInfo: ReactComponentInfo = candidateInfo; + logComponentInfo( + response, + root, componentInfo, trackIdx, - startTime, - endTime, + time, + componentEndTime, childrenEndTime, - response._rootEnvironmentName, - root.reason, + isLastComponent, ); - } else { - logComponentRender( - componentInfo, + componentEndTime = time; // The end time of previous component is the start time of the next. + // Track the root most component of the result for deduping logging. + result.component = componentInfo; + isLastComponent = false; + } else if (candidateInfo.awaited) { + if (endTime > childrenEndTime) { + childrenEndTime = endTime; + } + // $FlowFixMe: Refined. + const asyncInfo: ReactAsyncInfo = candidateInfo; + logComponentAwait( + asyncInfo, trackIdx, - startTime, + time, endTime, - childrenEndTime, response._rootEnvironmentName, ); } - // Track the root most component of the result for deduping logging. - result.component = componentInfo; - // Set the end time of the previous component to the start of the previous. - endTime = startTime; - } - isLastComponent = false; - } else if (info.awaited && i > 0 && i < debugInfo.length - 2) { - // $FlowFixMe: Refined. - const asyncInfo: ReactAsyncInfo = info; - const startTimeInfo = debugInfo[i - 1]; - const endTimeInfo = debugInfo[i + 1]; - if ( - typeof startTimeInfo.time === 'number' && - typeof endTimeInfo.time === 'number' - ) { - const awaitStartTime = startTimeInfo.time; - const awaitEndTime = endTimeInfo.time; - logComponentAwait( - asyncInfo, - trackIdx, - awaitStartTime, - awaitEndTime, - response._rootEnvironmentName, - ); } } + endTime = time; // The end time of the next entry is this time. + endTimeIdx = i; } } result.endTime = childrenEndTime; diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index 6a0a37b787d34..40de7ca51e418 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -18,13 +18,10 @@ import type { import type {LazyComponent} from 'react/src/ReactLazy'; import type {TemporaryReferenceSet} from './ReactFlightTemporaryReferences'; -import {enableRenderableContext} from 'shared/ReactFeatureFlags'; - import { REACT_ELEMENT_TYPE, REACT_LAZY_TYPE, REACT_CONTEXT_TYPE, - REACT_PROVIDER_TYPE, getIteratorFn, ASYNC_ITERATOR, } from 'shared/ReactSymbols'; @@ -699,10 +696,7 @@ export function processReply( return serializeTemporaryReferenceMarker(); } if (__DEV__) { - if ( - (value: any).$$typeof === - (enableRenderableContext ? REACT_CONTEXT_TYPE : REACT_PROVIDER_TYPE) - ) { + if ((value: any).$$typeof === REACT_CONTEXT_TYPE) { console.error( 'React Context Providers cannot be passed to Server Functions from the Client.%s', describeObjectForErrorMessage(parent, key), diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index ef9864200588a..49968b33590d9 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -320,7 +320,6 @@ describe('ReactFlight', () => { name: 'Greeting', env: 'Server', key: null, - owner: null, stack: ' in Object. (at **)', props: { firstName: 'Seb', @@ -364,7 +363,6 @@ describe('ReactFlight', () => { name: 'Greeting', env: 'Server', key: null, - owner: null, stack: ' in Object. (at **)', props: { firstName: 'Seb', @@ -2812,7 +2810,6 @@ describe('ReactFlight', () => { name: 'ServerComponent', env: 'Server', key: null, - owner: null, stack: ' in Object. (at **)', props: { transport: expect.arrayContaining([]), @@ -2829,16 +2826,15 @@ describe('ReactFlight', () => { expect(getDebugInfo(thirdPartyChildren[0])).toEqual( __DEV__ ? [ - {time: 14}, + {time: 22}, // Clamped to the start { name: 'ThirdPartyComponent', env: 'third-party', key: null, - owner: null, stack: ' in Object. (at **)', props: {}, }, - {time: 15}, + {time: 22}, {time: 23}, // This last one is when the promise resolved into the first party. ] : undefined, @@ -2846,32 +2842,30 @@ describe('ReactFlight', () => { expect(getDebugInfo(thirdPartyChildren[1])).toEqual( __DEV__ ? [ - {time: 16}, + {time: 22}, // Clamped to the start { name: 'ThirdPartyLazyComponent', env: 'third-party', key: null, - owner: null, stack: ' in myLazy (at **)\n in lazyInitializer (at **)', props: {}, }, - {time: 17}, + {time: 22}, ] : undefined, ); expect(getDebugInfo(thirdPartyChildren[2])).toEqual( __DEV__ ? [ - {time: 12}, + {time: 22}, { name: 'ThirdPartyFragmentComponent', env: 'third-party', key: '3', - owner: null, stack: ' in Object. (at **)', props: {}, }, - {time: 13}, + {time: 22}, ] : undefined, ); @@ -2941,7 +2935,6 @@ describe('ReactFlight', () => { name: 'ServerComponent', env: 'Server', key: null, - owner: null, stack: ' in Object. (at **)', props: { transport: expect.arrayContaining([]), @@ -2961,7 +2954,6 @@ describe('ReactFlight', () => { name: 'Keyed', env: 'Server', key: 'keyed', - owner: null, stack: ' in ServerComponent (at **)', props: { children: {}, @@ -2975,16 +2967,15 @@ describe('ReactFlight', () => { expect(getDebugInfo(thirdPartyFragment.props.children)).toEqual( __DEV__ ? [ - {time: 12}, + {time: 19}, // Clamp to the start { name: 'ThirdPartyAsyncIterableComponent', env: 'third-party', key: null, - owner: null, stack: ' in Object. (at **)', props: {}, }, - {time: 13}, + {time: 19}, ] : undefined, ); @@ -3000,6 +2991,64 @@ describe('ReactFlight', () => { ); }); + // @gate !__DEV__ || enableComponentPerformanceTrack + it('preserves debug info for server-to-server through use()', async () => { + function ThirdPartyComponent() { + return 'hi'; + } + + function ServerComponent({transport}) { + // This is a Server Component that receives other Server Components from a third party. + const text = ReactServer.use(ReactNoopFlightClient.read(transport)); + return
{text.toUpperCase()}
; + } + + const thirdPartyTransport = ReactNoopFlightServer.render( + , + { + environmentName: 'third-party', + }, + ); + + const transport = ReactNoopFlightServer.render( + , + ); + + await act(async () => { + const promise = ReactNoopFlightClient.read(transport); + expect(getDebugInfo(promise)).toEqual( + __DEV__ + ? [ + {time: 16}, + { + name: 'ServerComponent', + env: 'Server', + key: null, + stack: ' in Object. (at **)', + props: { + transport: expect.arrayContaining([]), + }, + }, + {time: 16}, + { + name: 'ThirdPartyComponent', + env: 'third-party', + key: null, + stack: ' in Object. (at **)', + props: {}, + }, + {time: 16}, + {time: 17}, + ] + : undefined, + ); + const result = await promise; + ReactNoop.render(result); + }); + + expect(ReactNoop).toMatchRenderedOutput(
HI
); + }); + it('preserves error stacks passed through server-to-server with source maps', async () => { async function ServerComponent({transport}) { // This is a Server Component that receives other Server Components from a third party. @@ -3137,7 +3186,6 @@ describe('ReactFlight', () => { name: 'Component', env: 'A', key: null, - owner: null, stack: ' in Object. (at **)', props: {}, }, @@ -3325,7 +3373,6 @@ describe('ReactFlight', () => { name: 'Greeting', env: 'Server', key: null, - owner: null, stack: ' in Object. (at **)', props: { firstName: 'Seb', diff --git a/packages/react-client/src/forks/ReactFlightClientConfig.dom-node-webstreams.js b/packages/react-client/src/forks/ReactFlightClientConfig.dom-node-webstreams.js deleted file mode 100644 index eb9ad28d46fa3..0000000000000 --- a/packages/react-client/src/forks/ReactFlightClientConfig.dom-node-webstreams.js +++ /dev/null @@ -1,18 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -export {default as rendererVersion} from 'shared/ReactVersion'; -export const rendererPackageName = 'react-server-dom-webpack'; - -export * from 'react-client/src/ReactFlightClientStreamConfigWeb'; -export * from 'react-client/src/ReactClientConsoleConfigServer'; -export * from 'react-server-dom-webpack/src/client/ReactFlightClientConfigBundlerNode'; -export * from 'react-server-dom-webpack/src/client/ReactFlightClientConfigTargetWebpackServer'; -export * from 'react-dom-bindings/src/shared/ReactFlightClientConfigDOM'; -export const usedWithSSR = true; diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/ComponentsSettings.js b/packages/react-devtools-shared/src/devtools/views/Settings/ComponentsSettings.js index 4309b0e5fbe4d..106538cad3ab2 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/ComponentsSettings.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/ComponentsSettings.js @@ -340,30 +340,35 @@ export default function ComponentsSettings({ ); return ( -
- +
+
+ +
- +
+ +