From 1eaccd8285f0bd40407705a9356391a171adf3b1 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Fri, 16 Aug 2024 21:08:20 +0200 Subject: [PATCH 1/5] [Fax] Make `react-markup` publishable via scripts (#30722) --- ReactVersions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactVersions.js b/ReactVersions.js index 736d6c8f54717..731f5f336270e 100644 --- a/ReactVersions.js +++ b/ReactVersions.js @@ -52,7 +52,7 @@ const stablePackages = { // These packages do not exist in the @canary or @latest channel, only // @experimental. We don't use semver, just the commit sha, so this is just a // list of package names instead of a map. -const experimentalPackages = []; +const experimentalPackages = ['react-markup']; module.exports = { ReactVersion, From 7b41cdc093c7a28a089e2c402cbe98cac68de509 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 16 Aug 2024 14:21:57 -0700 Subject: [PATCH 2/5] [Flight][Static] Implement halting a prerender behind enableHalt (#30705) enableHalt turns on a mode for flight prerenders where aborts are treated like infinitely stalled outcomes while still completing the prerender. For regular tasks we simply serialize the slot as a promise that never settles. For ReadableStream, Blob, and Async Iterators we just never advance the serialization so they remain unfinished when consumed on the client. When enableHalt is turned on aborts of prerenders will halt rather than error. The abort reason is forwarded to the upstream produces of the aforementioned async iterators, blobs, and ReadableStreams. In the future if we expose a signal that you can consume from within a render to cancel additional work the abort reason will also be forwarded there --- .../src/server/ReactFlightDOMServerNode.js | 17 +- .../src/server/ReactFlightDOMServerBrowser.js | 17 +- .../src/server/ReactFlightDOMServerEdge.js | 17 +- .../src/server/ReactFlightDOMServerNode.js | 17 +- .../src/__tests__/ReactFlightDOM-test.js | 134 +++++++ .../__tests__/ReactFlightDOMBrowser-test.js | 115 +++++- .../src/__tests__/ReactFlightDOMEdge-test.js | 341 +++++++++++++----- .../src/__tests__/ReactFlightDOMNode-test.js | 133 +++++++ .../src/server/ReactFlightDOMServerBrowser.js | 17 +- .../src/server/ReactFlightDOMServerEdge.js | 17 +- .../src/server/ReactFlightDOMServerNode.js | 17 +- .../react-server/src/ReactFlightServer.js | 124 +++++-- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 2 + 19 files changed, 855 insertions(+), 120 deletions(-) diff --git a/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js index bb65ef4b659a7..1434d17015a54 100644 --- a/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js @@ -20,12 +20,15 @@ import type {Thenable} from 'shared/ReactTypes'; import {Readable} from 'stream'; +import {enableHalt} from 'shared/ReactFeatureFlags'; + import { createRequest, startWork, startFlowing, stopFlowing, abort, + halt, } from 'react-server/src/ReactFlightServer'; import { @@ -187,10 +190,20 @@ function prerenderToNodeStream( if (options && options.signal) { const signal = options.signal; if (signal.aborted) { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } } else { const listener = () => { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } signal.removeEventListener('abort', listener); }; signal.addEventListener('abort', listener); diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js index ef980764942d7..56c3d5b71f432 100644 --- a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js @@ -12,12 +12,15 @@ import type {Thenable} from 'shared/ReactTypes'; import type {ClientManifest} from './ReactFlightServerConfigTurbopackBundler'; import type {ServerManifest} from 'react-client/src/ReactFlightClientConfig'; +import {enableHalt} from 'shared/ReactFeatureFlags'; + import { createRequest, startWork, startFlowing, stopFlowing, abort, + halt, } from 'react-server/src/ReactFlightServer'; import { @@ -146,10 +149,20 @@ function prerender( if (options && options.signal) { const signal = options.signal; if (signal.aborted) { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } } else { const listener = () => { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } signal.removeEventListener('abort', listener); }; signal.addEventListener('abort', listener); diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js index ef980764942d7..56c3d5b71f432 100644 --- a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js +++ b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js @@ -12,12 +12,15 @@ import type {Thenable} from 'shared/ReactTypes'; import type {ClientManifest} from './ReactFlightServerConfigTurbopackBundler'; import type {ServerManifest} from 'react-client/src/ReactFlightClientConfig'; +import {enableHalt} from 'shared/ReactFeatureFlags'; + import { createRequest, startWork, startFlowing, stopFlowing, abort, + halt, } from 'react-server/src/ReactFlightServer'; import { @@ -146,10 +149,20 @@ function prerender( if (options && options.signal) { const signal = options.signal; if (signal.aborted) { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } } else { const listener = () => { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } signal.removeEventListener('abort', listener); }; signal.addEventListener('abort', listener); diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js index e484d4b7e77d5..f9b0c163b2154 100644 --- a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js @@ -20,12 +20,15 @@ import type {Thenable} from 'shared/ReactTypes'; import {Readable} from 'stream'; +import {enableHalt} from 'shared/ReactFeatureFlags'; + import { createRequest, startWork, startFlowing, stopFlowing, abort, + halt, } from 'react-server/src/ReactFlightServer'; import { @@ -189,10 +192,20 @@ function prerenderToNodeStream( if (options && options.signal) { const signal = options.signal; if (signal.aborted) { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } } else { const listener = () => { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } signal.removeEventListener('abort', listener); }; signal.addEventListener('abort', listener); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index faaf8aef01b0d..6a0ce0152b704 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -2722,4 +2722,138 @@ describe('ReactFlightDOM', () => { await readInto(container, fizzReadable); expect(getMeaningfulChildren(container)).toEqual(
hello world
); }); + + // @gate enableHalt + it('serializes unfinished tasks with infinite promises when aborting a prerender', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + function App() { + return ( +
+ + + +
+ ); + } + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const controller = new AbortController(); + const {pendingResult} = await serverAct(async () => { + // destructure trick to avoid the act scope from awaiting the returned value + return { + pendingResult: ReactServerDOMStaticServer.prerenderToNodeStream( + , + webpackMap, + { + signal: controller.signal, + }, + ), + }; + }); + + controller.abort(); + resolveGreeting(); + const {prelude} = await pendingResult; + + const preludeWeb = Readable.toWeb(prelude); + const response = ReactServerDOMClient.createFromReadableStream(preludeWeb); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const errors = []; + let abortFizz; + await serverAct(async () => { + const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onError(error) { + errors.push(error); + }, + }, + ); + pipe(fizzWritable); + abortFizz = abort; + }); + + await serverAct(() => { + abortFizz('boom'); + }); + + expect(errors).toEqual(['boom']); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual(
loading...
); + }); + + // @gate enableHalt + it('will leave async iterables in an incomplete state when halting', async () => { + let resolve; + const wait = new Promise(r => (resolve = r)); + const errors = []; + + const multiShotIterable = { + async *[Symbol.asyncIterator]() { + yield {hello: 'A'}; + await wait; + yield {hi: 'B'}; + return 'C'; + }, + }; + + const controller = new AbortController(); + const {pendingResult} = await serverAct(() => { + return { + pendingResult: ReactServerDOMStaticServer.prerenderToNodeStream( + { + multiShotIterable, + }, + {}, + { + onError(x) { + errors.push(x); + }, + signal: controller.signal, + }, + ), + }; + }); + + controller.abort(); + await serverAct(() => resolve()); + + const {prelude} = await pendingResult; + + const result = await ReactServerDOMClient.createFromReadableStream( + Readable.toWeb(prelude), + ); + + const iterator = result.multiShotIterable[Symbol.asyncIterator](); + + expect(await iterator.next()).toEqual({ + value: {hello: 'A'}, + done: false, + }); + + const race = Promise.race([ + iterator.next(), + new Promise(r => setTimeout(() => r('timeout'), 10)), + ]); + + await 1; + jest.advanceTimersByTime('100'); + expect(await race).toBe('timeout'); + }); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index db8edf7ad6831..7bbfea1484bed 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -29,6 +29,7 @@ let ReactDOM; let ReactDOMClient; let ReactDOMFizzServer; let ReactServerDOMServer; +let ReactServerDOMStaticServer; let ReactServerDOMClient; let Suspense; let use; @@ -60,7 +61,13 @@ describe('ReactFlightDOMBrowser', () => { serverExports = WebpackMock.serverExports; webpackMap = WebpackMock.webpackMap; webpackServerMap = WebpackMock.webpackServerMap; - ReactServerDOMServer = require('react-server-dom-webpack/server.browser'); + ReactServerDOMServer = require('react-server-dom-webpack/server'); + if (__EXPERIMENTAL__) { + jest.mock('react-server-dom-webpack/static', () => + require('react-server-dom-webpack/static.browser'), + ); + ReactServerDOMStaticServer = require('react-server-dom-webpack/static'); + } __unmockReact(); jest.resetModules(); @@ -2332,4 +2339,110 @@ describe('ReactFlightDOMBrowser', () => { expect(error.digest).toBe('aborted'); expect(errors).toEqual([reason]); }); + + // @gate experimental + it('can prerender', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + function App() { + return ( +
+ +
+ ); + } + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const {pendingResult} = await serverAct(async () => { + // destructure trick to avoid the act scope from awaiting the returned value + return { + pendingResult: ReactServerDOMStaticServer.prerender( + , + webpackMap, + ), + }; + }); + + resolveGreeting(); + const {prelude} = await pendingResult; + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream( + passThrough(prelude), + ); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + expect(container.innerHTML).toBe('
hello world
'); + }); + + // @gate enableHalt + it('serializes unfinished tasks with infinite promises when aborting a prerender', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + function App() { + return ( +
+ + + +
+ ); + } + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const controller = new AbortController(); + const {pendingResult} = await serverAct(async () => { + // destructure trick to avoid the act scope from awaiting the returned value + return { + pendingResult: ReactServerDOMStaticServer.prerender( + , + webpackMap, + { + signal: controller.signal, + }, + ), + }; + }); + + controller.abort(); + resolveGreeting(); + const {prelude} = await pendingResult; + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream( + passThrough(prelude), + ); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + + expect(container.innerHTML).toBe('
loading...
'); + }); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js index ffef621e9761b..b38c33dc7761b 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js @@ -23,9 +23,9 @@ if (typeof File === 'undefined' || typeof FormData === 'undefined') { // Patch for Edge environments for global scope global.AsyncLocalStorage = require('async_hooks').AsyncLocalStorage; -// Don't wait before processing work on the server. -// TODO: we can replace this with FlightServer.act(). -global.setTimeout = cb => cb(); +const { + patchMessageChannel, +} = require('../../../../scripts/jest/patchMessageChannel'); let serverExports; let clientExports; @@ -36,8 +36,11 @@ let React; let ReactServer; let ReactDOMServer; let ReactServerDOMServer; +let ReactServerDOMStaticServer; let ReactServerDOMClient; let use; +let ReactServerScheduler; +let reactServerAct; function normalizeCodeLocInfo(str) { return ( @@ -52,6 +55,10 @@ describe('ReactFlightDOMEdge', () => { beforeEach(() => { jest.resetModules(); + ReactServerScheduler = require('scheduler'); + patchMessageChannel(ReactServerScheduler); + reactServerAct = require('internal-test-utils').act; + // Simulate the condition resolution jest.mock('react', () => require('react/react.react-server')); jest.mock('react-server-dom-webpack/server', () => @@ -68,6 +75,12 @@ describe('ReactFlightDOMEdge', () => { ReactServer = require('react'); ReactServerDOMServer = require('react-server-dom-webpack/server'); + if (__EXPERIMENTAL__) { + jest.mock('react-server-dom-webpack/static', () => + require('react-server-dom-webpack/static.edge'), + ); + ReactServerDOMStaticServer = require('react-server-dom-webpack/static'); + } jest.resetModules(); __unmockReact(); @@ -81,6 +94,17 @@ describe('ReactFlightDOMEdge', () => { use = React.use; }); + async function serverAct(callback) { + let maybePromise; + await reactServerAct(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + function passThrough(stream) { // Simulate more realistic network by splitting up and rejoining some chunks. // This lets us test that we don't accidentally rely on particular bounds of the chunks. @@ -174,9 +198,8 @@ describe('ReactFlightDOMEdge', () => { return ; } - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(, webpackMap), ); const response = ReactServerDOMClient.createFromReadableStream(stream, { ssrManifest: { @@ -189,8 +212,8 @@ describe('ReactFlightDOMEdge', () => { return use(response); } - const ssrStream = await ReactDOMServer.renderToReadableStream( - , + const ssrStream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), ); const result = await readResult(ssrStream); expect(result).toEqual('Client Component'); @@ -200,10 +223,12 @@ describe('ReactFlightDOMEdge', () => { const testString = '"\n\t'.repeat(500) + '🙃'; const testString2 = 'hello'.repeat(400); - const stream = ReactServerDOMServer.renderToReadableStream({ - text: testString, - text2: testString2, - }); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream({ + text: testString, + text2: testString2, + }), + ); const [stream1, stream2] = passThrough(stream).tee(); const serializedContent = await readResult(stream1); @@ -234,7 +259,9 @@ describe('ReactFlightDOMEdge', () => { with: {many: 'properties in it'}, }; const props = {root:
{new Array(30).fill(obj)}
}; - const stream = ReactServerDOMServer.renderToReadableStream(props); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(props), + ); const [stream1, stream2] = passThrough(stream).tee(); const serializedContent = await readResult(stream1); @@ -302,7 +329,9 @@ describe('ReactFlightDOMEdge', () => { ); const resolvedChildren = new Array(30).fill(str); - const stream = ReactServerDOMServer.renderToReadableStream(children); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(children), + ); const [stream1, stream2] = passThrough(stream).tee(); const serializedContent = await readResult(stream1); @@ -318,7 +347,9 @@ describe('ReactFlightDOMEdge', () => { }); // Use the SSR render to resolve any lazy elements - const ssrStream = await ReactDOMServer.renderToReadableStream(model); + const ssrStream = await serverAct(() => + ReactDOMServer.renderToReadableStream(model), + ); // Should still match the result when parsed const result = await readResult(ssrStream); expect(result).toEqual(resolvedChildren.join('')); @@ -370,22 +401,28 @@ describe('ReactFlightDOMEdge', () => { const resolvedChildren = new Array(30).fill( '
this is a long return value
', ); - const stream = ReactServerDOMServer.renderToReadableStream(children); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(children), + ); const [stream1, stream2] = passThrough(stream).tee(); const serializedContent = await readResult(stream1); expect(serializedContent.length).toBeLessThan(__DEV__ ? 605 : 400); expect(timesRendered).toBeLessThan(5); - const model = await ReactServerDOMClient.createFromReadableStream(stream2, { - ssrManifest: { - moduleMap: null, - moduleLoading: null, - }, - }); + const model = await serverAct(() => + ReactServerDOMClient.createFromReadableStream(stream2, { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }), + ); // Use the SSR render to resolve any lazy elements - const ssrStream = await ReactDOMServer.renderToReadableStream(model); + const ssrStream = await serverAct(() => + ReactDOMServer.renderToReadableStream(model), + ); // Should still match the result when parsed const result = await readResult(ssrStream); expect(result).toEqual(resolvedChildren.join('')); @@ -398,8 +435,10 @@ describe('ReactFlightDOMEdge', () => { } return
Fin
; } - const stream = ReactServerDOMServer.renderToReadableStream( - , + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + ), ); const serializedContent = await readResult(stream); const expectedDebugInfoSize = __DEV__ ? 300 * 20 : 0; @@ -426,8 +465,8 @@ describe('ReactFlightDOMEdge', () => { new BigUint64Array(buffer, 0), new DataView(buffer, 3), ]; - const stream = passThrough( - ReactServerDOMServer.renderToReadableStream(buffers), + const stream = await serverAct(() => + passThrough(ReactServerDOMServer.renderToReadableStream(buffers)), ); const result = await ReactServerDOMClient.createFromReadableStream(stream, { ssrManifest: { @@ -446,8 +485,8 @@ describe('ReactFlightDOMEdge', () => { const blob = new Blob([bytes, bytes], { type: 'application/x-test', }); - const stream = passThrough( - ReactServerDOMServer.renderToReadableStream(blob), + const stream = await serverAct(() => + passThrough(ReactServerDOMServer.renderToReadableStream(blob)), ); const result = await ReactServerDOMClient.createFromReadableStream(stream, { ssrManifest: { @@ -476,8 +515,8 @@ describe('ReactFlightDOMEdge', () => { expect(formData.get('file') instanceof File).toBe(true); expect(formData.get('file').name).toBe('filename.test'); - const stream = passThrough( - ReactServerDOMServer.renderToReadableStream(formData), + const stream = await serverAct(() => + passThrough(ReactServerDOMServer.renderToReadableStream(formData)), ); const result = await ReactServerDOMClient.createFromReadableStream(stream, { ssrManifest: { @@ -507,8 +546,8 @@ describe('ReactFlightDOMEdge', () => { const map = new Map(); map.set('value', awaitedValue); - const stream = passThrough( - ReactServerDOMServer.renderToReadableStream(map, webpackMap), + const stream = await serverAct(() => + passThrough(ReactServerDOMServer.renderToReadableStream(map, webpackMap)), ); // Parsing the root blocks because the module hasn't loaded yet @@ -549,16 +588,18 @@ describe('ReactFlightDOMEdge', () => { }, }); - const stream = passThrough( - ReactServerDOMServer.renderToReadableStream(s, webpackMap), + const stream = await serverAct(() => + passThrough(ReactServerDOMServer.renderToReadableStream(s, webpackMap)), ); - const result = await ReactServerDOMClient.createFromReadableStream(stream, { - ssrManifest: { - moduleMap: null, - moduleLoading: null, - }, - }); + const result = await serverAct(() => + ReactServerDOMClient.createFromReadableStream(stream, { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }), + ); const reader = result.getReader(); @@ -589,20 +630,24 @@ describe('ReactFlightDOMEdge', () => { }, }; - const stream = passThrough( - ReactServerDOMServer.renderToReadableStream( - multiShotIterable, - webpackMap, + const stream = await serverAct(() => + passThrough( + ReactServerDOMServer.renderToReadableStream( + multiShotIterable, + webpackMap, + ), ), ); // Parsing the root blocks because the module hasn't loaded yet - const result = await ReactServerDOMClient.createFromReadableStream(stream, { - ssrManifest: { - moduleMap: null, - moduleLoading: null, - }, - }); + const result = await serverAct(() => + ReactServerDOMClient.createFromReadableStream(stream, { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }), + ); const iterator = result[Symbol.asyncIterator](); @@ -635,9 +680,11 @@ describe('ReactFlightDOMEdge', () => { }, }; - const stream = ReactServerDOMServer.renderToReadableStream({ - iterable, - }); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream({ + iterable, + }), + ); const [stream1, stream2] = passThrough(stream).tee(); const serializedContent = await readResult(stream1); @@ -728,7 +775,9 @@ describe('ReactFlightDOMEdge', () => { }, }); - const stream = ReactServerDOMServer.renderToReadableStream(s, {}); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(s, {}), + ); const [stream1, stream2] = passThrough(stream).tee(); @@ -785,7 +834,9 @@ describe('ReactFlightDOMEdge', () => { }, }); - const stream = ReactServerDOMServer.renderToReadableStream(s, {}); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(s, {}), + ); const [stream1, stream2] = passThrough(stream).tee(); @@ -841,23 +892,21 @@ describe('ReactFlightDOMEdge', () => { greeting: ReactServer.createElement(Greeting, {firstName: 'Seb'}), }; - const stream = ReactServerDOMServer.renderToReadableStream( - model, - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(model, webpackMap), ); - const rootModel = await ReactServerDOMClient.createFromReadableStream( - stream, - { + const rootModel = await serverAct(() => + ReactServerDOMClient.createFromReadableStream(stream, { ssrManifest: { moduleMap: null, moduleLoading: null, }, - }, + }), ); - const ssrStream = await ReactDOMServer.renderToReadableStream( - rootModel.greeting, + const ssrStream = await serverAct(() => + ReactDOMServer.renderToReadableStream(rootModel.greeting), ); const result = await readResult(ssrStream); expect(result).toEqual('Hello, Seb'); @@ -916,13 +965,15 @@ describe('ReactFlightDOMEdge', () => { return ReactServer.createElement('span', null, 'hi'); } - const stream = ReactServerDOMServer.renderToReadableStream( - ReactServer.createElement( - 'div', - null, - ReactServer.createElement(Foo, null), + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + ReactServer.createElement( + 'div', + null, + ReactServer.createElement(Foo, null), + ), + webpackMap, ), - webpackMap, ); await readResult(stream); @@ -943,35 +994,31 @@ describe('ReactFlightDOMEdge', () => { root: ReactServer.createElement(Erroring), }; - const stream = ReactServerDOMServer.renderToReadableStream( - model, - webpackMap, - { + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(model, webpackMap, { onError() {}, - }, + }), ); - const rootModel = await ReactServerDOMClient.createFromReadableStream( - stream, - { + const rootModel = await serverAct(() => + ReactServerDOMClient.createFromReadableStream(stream, { ssrManifest: { moduleMap: null, moduleLoading: null, }, - }, + }), ); const errors = []; - const result = ReactDOMServer.renderToReadableStream( -
{rootModel.root}
, - { + const result = serverAct(() => + ReactDOMServer.renderToReadableStream(
{rootModel.root}
, { onError(error, {componentStack}) { errors.push({ error, componentStack: normalizeCodeLocInfo(componentStack), }); }, - }, + }), ); const theError = new Error('my error'); @@ -1000,4 +1047,130 @@ describe('ReactFlightDOMEdge', () => { }, ]); }); + + // @gate experimental + it('can prerender', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + function App() { + return ( +
+ +
+ ); + } + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const {pendingResult} = await serverAct(async () => { + // destructure trick to avoid the act scope from awaiting the returned value + return { + pendingResult: ReactServerDOMStaticServer.prerender( + , + webpackMap, + ), + }; + }); + + resolveGreeting(); + const {prelude} = await pendingResult; + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream(prelude, { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }); + // Use the SSR render to resolve any lazy elements + const ssrStream = await serverAct(() => + ReactDOMServer.renderToReadableStream( + React.createElement(ClientRoot, {response}), + ), + ); + // Should still match the result when parsed + const result = await readResult(ssrStream); + expect(result).toBe('
hello world
'); + }); + + // @gate enableHalt + it('serializes unfinished tasks with infinite promises when aborting a prerender', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + function App() { + return ( +
+ + + +
+ ); + } + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const controller = new AbortController(); + const {pendingResult} = await serverAct(async () => { + // destructure trick to avoid the act scope from awaiting the returned value + return { + pendingResult: ReactServerDOMStaticServer.prerender( + , + webpackMap, + { + signal: controller.signal, + }, + ), + }; + }); + + controller.abort(); + resolveGreeting(); + const {prelude} = await pendingResult; + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream(prelude, { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }); + const fizzController = new AbortController(); + const errors = []; + const ssrStream = await serverAct(() => + ReactDOMServer.renderToReadableStream( + React.createElement(ClientRoot, {response}), + { + signal: fizzController.signal, + onError(error) { + errors.push(error); + }, + }, + ), + ); + fizzController.abort('boom'); + expect(errors).toEqual(['boom']); + // Should still match the result when parsed + const result = await readResult(ssrStream); + const div = document.createElement('div'); + div.innerHTML = result; + expect(div.textContent).toBe('loading...'); + }); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js index 2de34cc1c493f..fbe32d2f1f697 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js @@ -20,7 +20,9 @@ let webpackModules; let webpackModuleLoading; let React; let ReactDOMServer; +let ReactServer; let ReactServerDOMServer; +let ReactServerDOMStaticServer; let ReactServerDOMClient; let Stream; let use; @@ -45,7 +47,14 @@ describe('ReactFlightDOMNode', () => { jest.mock('react-server-dom-webpack/server', () => require('react-server-dom-webpack/server.node'), ); + ReactServer = require('react'); ReactServerDOMServer = require('react-server-dom-webpack/server'); + if (__EXPERIMENTAL__) { + jest.mock('react-server-dom-webpack/static', () => + require('react-server-dom-webpack/static.node'), + ); + ReactServerDOMStaticServer = require('react-server-dom-webpack/static'); + } const WebpackMock = require('./utils/WebpackMock'); clientExports = WebpackMock.clientExports; @@ -378,4 +387,128 @@ describe('ReactFlightDOMNode', () => { expect(error.digest).toBe('aborted'); expect(errors).toEqual([reason]); }); + + // @gate experimental + it('can prerender', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + function App() { + return ( +
+ +
+ ); + } + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const {pendingResult} = await serverAct(async () => { + // destructure trick to avoid the act scope from awaiting the returned value + return { + pendingResult: ReactServerDOMStaticServer.prerenderToNodeStream( + , + webpackMap, + ), + }; + }); + + resolveGreeting(); + const {prelude} = await pendingResult; + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromNodeStream(prelude, { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }); + // Use the SSR render to resolve any lazy elements + const ssrStream = await serverAct(() => + ReactDOMServer.renderToPipeableStream( + React.createElement(ClientRoot, {response}), + ), + ); + // Should still match the result when parsed + const result = await readResult(ssrStream); + expect(result).toBe('
hello world
'); + }); + + // @gate enableHalt + it('serializes unfinished tasks with infinite promises when aborting a prerender', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + function App() { + return ( +
+ + + +
+ ); + } + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const controller = new AbortController(); + const {pendingResult} = await serverAct(async () => { + // destructure trick to avoid the act scope from awaiting the returned value + return { + pendingResult: ReactServerDOMStaticServer.prerenderToNodeStream( + , + webpackMap, + { + signal: controller.signal, + }, + ), + }; + }); + + controller.abort(); + resolveGreeting(); + const {prelude} = await pendingResult; + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromNodeStream(prelude, { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }); + const errors = []; + const ssrStream = await serverAct(() => + ReactDOMServer.renderToPipeableStream( + React.createElement(ClientRoot, {response}), + { + onError(error) { + errors.push(error); + }, + }, + ), + ); + ssrStream.abort('boom'); + expect(errors).toEqual(['boom']); + // Should still match the result when parsed + const result = await readResult(ssrStream); + const div = document.createElement('div'); + div.innerHTML = result; + expect(div.textContent).toBe('loading...'); + }); }); diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js index a4e0c3bef693b..95e7f770428a3 100644 --- a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js @@ -12,12 +12,15 @@ import type {Thenable} from 'shared/ReactTypes'; import type {ClientManifest} from './ReactFlightServerConfigWebpackBundler'; import type {ServerManifest} from 'react-client/src/ReactFlightClientConfig'; +import {enableHalt} from 'shared/ReactFeatureFlags'; + import { createRequest, startWork, startFlowing, stopFlowing, abort, + halt, } from 'react-server/src/ReactFlightServer'; import { @@ -146,10 +149,20 @@ function prerender( if (options && options.signal) { const signal = options.signal; if (signal.aborted) { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } } else { const listener = () => { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } signal.removeEventListener('abort', listener); }; signal.addEventListener('abort', listener); diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js index a4e0c3bef693b..95e7f770428a3 100644 --- a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js +++ b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js @@ -12,12 +12,15 @@ import type {Thenable} from 'shared/ReactTypes'; import type {ClientManifest} from './ReactFlightServerConfigWebpackBundler'; import type {ServerManifest} from 'react-client/src/ReactFlightClientConfig'; +import {enableHalt} from 'shared/ReactFeatureFlags'; + import { createRequest, startWork, startFlowing, stopFlowing, abort, + halt, } from 'react-server/src/ReactFlightServer'; import { @@ -146,10 +149,20 @@ function prerender( if (options && options.signal) { const signal = options.signal; if (signal.aborted) { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } } else { const listener = () => { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } signal.removeEventListener('abort', listener); }; signal.addEventListener('abort', listener); diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js index 1506259476703..1d8d6ea9ef743 100644 --- a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js @@ -20,12 +20,15 @@ import type {Thenable} from 'shared/ReactTypes'; import {Readable} from 'stream'; +import {enableHalt} from 'shared/ReactFeatureFlags'; + import { createRequest, startWork, startFlowing, stopFlowing, abort, + halt, } from 'react-server/src/ReactFlightServer'; import { @@ -189,10 +192,20 @@ function prerenderToNodeStream( if (options && options.signal) { const signal = options.signal; if (signal.aborted) { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } } else { const listener = () => { - abort(request, (signal: any).reason); + const reason = (signal: any).reason; + if (enableHalt) { + halt(request, reason); + } else { + abort(request, reason); + } signal.removeEventListener('abort', listener); }; signal.addEventListener('abort', listener); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index d8ba106d37e72..db0ee7b3cebad 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -16,6 +16,7 @@ import type {TemporaryReferenceSet} from './ReactFlightServerTemporaryReferences import { enableBinaryFlight, enablePostpone, + enableHalt, enableTaint, enableRefAsProp, enableServerComponentLogs, @@ -611,11 +612,15 @@ function serializeThenable( default: { if (request.status === ABORTING) { // We can no longer accept any resolved values - newTask.status = ABORTED; - const errorId: number = (request.fatalError: any); - const model = stringify(serializeByValueID(errorId)); - emitModelChunk(request, newTask.id, model); request.abortableTasks.delete(newTask); + newTask.status = ABORTED; + if (enableHalt && request.fatalError === haltSymbol) { + emitModelChunk(request, newTask.id, reusableInfinitePromiseModel); + } else { + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, newTask.id, model); + } return newTask.id; } if (typeof thenable.status === 'string') { @@ -748,23 +753,32 @@ function serializeReadableStream( } aborted = true; request.abortListeners.delete(error); - if ( + + let cancelWith: mixed; + if (enableHalt && request.fatalError === haltSymbol) { + cancelWith = reason; + } else if ( enablePostpone && typeof reason === 'object' && reason !== null && (reason: any).$$typeof === REACT_POSTPONE_TYPE ) { + cancelWith = reason; const postponeInstance: Postpone = (reason: any); logPostpone(request, postponeInstance.message, streamTask); emitPostponeChunk(request, streamTask.id, postponeInstance); + enqueueFlush(request); } else { + cancelWith = reason; const digest = logRecoverableError(request, reason, streamTask); emitErrorChunk(request, streamTask.id, digest, reason); + enqueueFlush(request); } - enqueueFlush(request); + // $FlowFixMe should be able to pass mixed - reader.cancel(reason).then(error, error); + reader.cancel(cancelWith).then(error, error); } + request.abortListeners.add(error); reader.read().then(progress, error); return serializeByValueID(streamTask.id); @@ -866,24 +880,30 @@ function serializeAsyncIterable( } aborted = true; request.abortListeners.delete(error); - if ( + let throwWith: mixed; + if (enableHalt && request.fatalError === haltSymbol) { + throwWith = reason; + } else if ( enablePostpone && typeof reason === 'object' && reason !== null && (reason: any).$$typeof === REACT_POSTPONE_TYPE ) { + throwWith = reason; const postponeInstance: Postpone = (reason: any); logPostpone(request, postponeInstance.message, streamTask); emitPostponeChunk(request, streamTask.id, postponeInstance); + enqueueFlush(request); } else { + throwWith = reason; const digest = logRecoverableError(request, reason, streamTask); emitErrorChunk(request, streamTask.id, digest, reason); + enqueueFlush(request); } - enqueueFlush(request); if (typeof (iterator: any).throw === 'function') { // The iterator protocol doesn't necessarily include this but a generator do. // $FlowFixMe should be able to pass mixed - iterator.throw(reason).then(error, error); + iterator.throw(throwWith).then(error, error); } } request.abortListeners.add(error); @@ -1798,6 +1818,7 @@ function serializeLazyID(id: number): string { function serializeInfinitePromise(): string { return '$@'; } +const reusableInfinitePromiseModel = stringify(serializeInfinitePromise()); function serializePromiseID(id: number): string { return '$@' + id.toString(16); @@ -2066,12 +2087,18 @@ function serializeBlob(request: Request, blob: Blob): string { } aborted = true; request.abortListeners.delete(error); - const digest = logRecoverableError(request, reason, newTask); - emitErrorChunk(request, newTask.id, digest, reason); - request.abortableTasks.delete(newTask); - enqueueFlush(request); + let cancelWith: mixed; + if (enableHalt && request.fatalError === haltSymbol) { + cancelWith = reason; + } else { + cancelWith = reason; + const digest = logRecoverableError(request, reason, newTask); + emitErrorChunk(request, newTask.id, digest, reason); + request.abortableTasks.delete(newTask); + enqueueFlush(request); + } // $FlowFixMe should be able to pass mixed - reader.cancel(reason).then(error, error); + reader.cancel(cancelWith).then(error, error); } request.abortListeners.add(error); @@ -2149,6 +2176,9 @@ function renderModel( if (typeof x.then === 'function') { if (request.status === ABORTING) { task.status = ABORTED; + if (enableHalt && request.fatalError === haltSymbol) { + return serializeInfinitePromise(); + } const errorId: number = (request.fatalError: any); if (wasReactNode) { return serializeLazyID(errorId); @@ -2202,6 +2232,9 @@ function renderModel( if (request.status === ABORTING) { task.status = ABORTED; + if (enableHalt && request.fatalError === haltSymbol) { + return serializeInfinitePromise(); + } const errorId: number = (request.fatalError: any); if (wasReactNode) { return serializeLazyID(errorId); @@ -3691,9 +3724,13 @@ function retryTask(request: Request, task: Task): void { if (request.status === ABORTING) { request.abortableTasks.delete(task); task.status = ABORTED; - const errorId: number = (request.fatalError: any); - const model = stringify(serializeByValueID(errorId)); - emitModelChunk(request, task.id, model); + if (enableHalt && request.fatalError === haltSymbol) { + emitModelChunk(request, task.id, reusableInfinitePromiseModel); + } else { + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, task.id, model); + } return; } // Something suspended again, let's pick it back up later. @@ -3715,9 +3752,13 @@ function retryTask(request: Request, task: Task): void { if (request.status === ABORTING) { request.abortableTasks.delete(task); task.status = ABORTED; - const errorId: number = (request.fatalError: any); - const model = stringify(serializeByValueID(errorId)); - emitModelChunk(request, task.id, model); + if (enableHalt && request.fatalError === haltSymbol) { + emitModelChunk(request, task.id, reusableInfinitePromiseModel); + } else { + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, task.id, model); + } return; } @@ -3795,6 +3836,15 @@ function abortTask(task: Task, request: Request, errorId: number): void { request.completedErrorChunks.push(processedChunk); } +function haltTask(task: Task, request: Request): void { + if (task.status === RENDERING) { + // This task will be aborted by the render + return; + } + task.status = ABORTED; + emitModelChunk(request, task.id, reusableInfinitePromiseModel); +} + function flushCompletedChunks( request: Request, destination: Destination, @@ -4012,3 +4062,35 @@ export function abort(request: Request, reason: mixed): void { fatalError(request, error); } } + +const haltSymbol = Symbol('halt'); + +// This is called to stop rendering without erroring. All unfinished work is represented Promises +// that never resolve. +export function halt(request: Request, reason: mixed): void { + try { + if (request.status === OPEN) { + request.status = ABORTING; + } + request.fatalError = haltSymbol; + const abortableTasks = request.abortableTasks; + // We have tasks to abort. We'll emit one error row and then emit a reference + // to that row from every row that's still remaining. + if (abortableTasks.size > 0) { + request.pendingChunks++; + abortableTasks.forEach(task => haltTask(task, request)); + abortableTasks.clear(); + } + const abortListeners = request.abortListeners; + if (abortListeners.size > 0) { + abortListeners.forEach(callback => callback(reason)); + abortListeners.clear(); + } + if (request.destination !== null) { + flushCompletedChunks(request, request.destination); + } + } catch (error) { + logRecoverableError(request, error, null); + fatalError(request, error); + } +} diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index b0286405a6cca..c5351d6d92631 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -87,6 +87,8 @@ export const enableTaint = __EXPERIMENTAL__; export const enablePostpone = __EXPERIMENTAL__; +export const enableHalt = __EXPERIMENTAL__; + /** * Switches the Fabric API from doing layout in commit work instead of complete work. */ diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 4eda27d16cfcb..3618aa70e7d67 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -58,6 +58,7 @@ export const enableFilterEmptyStringAttributesDOM = true; export const enableFizzExternalRuntime = true; export const enableFlightReadableStream = true; export const enableGetInspectorDataForInstanceInProduction = true; +export const enableHalt = false; export const enableInfiniteRenderLoopDetection = true; export const enableContextProfiling = false; export const enableLegacyCache = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 2a4421f41da0a..2aae8bd3d1c65 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -49,6 +49,7 @@ export const enableFilterEmptyStringAttributesDOM = true; export const enableFizzExternalRuntime = true; export const enableFlightReadableStream = true; export const enableGetInspectorDataForInstanceInProduction = false; +export const enableHalt = false; export const enableInfiniteRenderLoopDetection = true; export const enableLazyContextPropagation = false; export const enableContextProfiling = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 8778bf6558cb4..c44e7014fc444 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -25,6 +25,7 @@ export const enableFlightReadableStream = true; export const enableAsyncIterableChildren = false; export const enableTaint = true; export const enablePostpone = false; +export const enableHalt = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const disableIEWorkarounds = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 3a8a0c1d44cec..bc7ddf85acc03 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -40,6 +40,7 @@ export const enableFilterEmptyStringAttributesDOM = true; export const enableFizzExternalRuntime = true; export const enableFlightReadableStream = true; export const enableGetInspectorDataForInstanceInProduction = false; +export const enableHalt = false; export const enableInfiniteRenderLoopDetection = true; export const enableLazyContextPropagation = false; export const enableContextProfiling = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index eb801d7bac4b6..57f60c24aef45 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -25,6 +25,7 @@ export const enableFlightReadableStream = true; export const enableAsyncIterableChildren = false; export const enableTaint = true; export const enablePostpone = false; +export const enableHalt = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const disableIEWorkarounds = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 95cd1e5a6ebe6..465fa58590bcc 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -78,6 +78,8 @@ export const enableTaint = false; export const enablePostpone = false; +export const enableHalt = false; + export const enableContextProfiling = true; // TODO: www currently relies on this feature. It's disabled in open source. From e9a869fbb59a634fb9a39c9480ffa34970f35858 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 16 Aug 2024 17:39:50 -0400 Subject: [PATCH 3/5] [compiler] Run compiler pipeline on 'use no forget' This PR updates the babel plugin to continue the compilation pipeline as normal on components/hooks that have been opted out using a directive. Instead, we no longer emit the compiled function when the directive is present. Previously, we would skip over the entire pipeline. By continuing to enter the pipeline, we'll be able to detect if there are unused directives. The end result is: - (no change) 'use forget' will always opt into compilation - (new) 'use no forget' will opt out of compilation but continue to log errors without throwing them. This means that a Program containing multiple functions (some of which are opted out) will continue to compile correctly ghstack-source-id: 5bd85df2f81350cb2c1998a8761b8ed3fec32a40 Pull Request resolved: https://github.com/facebook/react/pull/30720 --- .../src/Entrypoint/Options.ts | 6 + .../src/Entrypoint/Program.ts | 146 ++++++++++-------- .../use-no-forget-with-no-errors.expect.md | 35 +++++ .../compiler/use-no-forget-with-no-errors.js | 10 ++ .../babel-plugin-react-compiler/src/index.ts | 1 + 5 files changed, 136 insertions(+), 62 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-forget-with-no-errors.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-forget-with-no-errors.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts index 722c62461d813..e97ececc2a137 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts @@ -165,6 +165,12 @@ export type LoggerEvent = fnLoc: t.SourceLocation | null; detail: Omit, 'suggestions'>; } + | { + kind: 'CompileSkip'; + fnLoc: t.SourceLocation | null; + reason: string; + loc: t.SourceLocation | null; + } | { kind: 'CompileSuccess'; fnLoc: t.SourceLocation | null; 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 499a4d124ea67..99ec1e04a65e4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -43,34 +43,23 @@ export type CompilerPass = { comments: Array; code: string | null; }; +const OPT_IN_DIRECTIVES = new Set(['use forget', 'use memo']); +export const OPT_OUT_DIRECTIVES = new Set(['use no forget', 'use no memo']); function findDirectiveEnablingMemoization( directives: Array, -): t.Directive | null { - for (const directive of directives) { - const directiveValue = directive.value.value; - if (directiveValue === 'use forget' || directiveValue === 'use memo') { - return directive; - } - } - return null; +): Array { + return directives.filter(directive => + OPT_IN_DIRECTIVES.has(directive.value.value), + ); } function findDirectiveDisablingMemoization( directives: Array, - options: PluginOptions, -): t.Directive | null { - for (const directive of directives) { - const directiveValue = directive.value.value; - if ( - (directiveValue === 'use no forget' || - directiveValue === 'use no memo') && - !options.ignoreUseNoForget - ) { - return directive; - } - } - return null; +): Array { + return directives.filter(directive => + OPT_OUT_DIRECTIVES.has(directive.value.value), + ); } function isCriticalError(err: unknown): boolean { @@ -102,7 +91,7 @@ export type CompileResult = { compiledFn: CodegenFunction; }; -function handleError( +function logError( err: unknown, pass: CompilerPass, fnLoc: t.SourceLocation | null, @@ -131,6 +120,13 @@ function handleError( }); } } +} +function handleError( + err: unknown, + pass: CompilerPass, + fnLoc: t.SourceLocation | null, +): void { + logError(err, pass, fnLoc); if ( pass.opts.panicThreshold === 'all_errors' || (pass.opts.panicThreshold === 'critical_errors' && isCriticalError(err)) || @@ -393,6 +389,17 @@ export function compileProgram( fn: BabelFn, fnType: ReactFunctionType, ): null | CodegenFunction => { + let optInDirectives: Array = []; + let optOutDirectives: Array = []; + if (fn.node.body.type === 'BlockStatement') { + optInDirectives = findDirectiveEnablingMemoization( + fn.node.body.directives, + ); + optOutDirectives = findDirectiveDisablingMemoization( + fn.node.body.directives, + ); + } + if (lintError != null) { /** * Note that Babel does not attach comment nodes to nodes; they are dangling off of the @@ -404,7 +411,11 @@ export function compileProgram( fn, ); if (suppressionsInFunction.length > 0) { - handleError(lintError, pass, fn.node.loc ?? null); + if (optOutDirectives.length > 0) { + logError(lintError, pass, fn.node.loc ?? null); + } else { + handleError(lintError, pass, fn.node.loc ?? null); + } } } @@ -430,11 +441,50 @@ export function compileProgram( prunedMemoValues: compiledFn.prunedMemoValues, }); } catch (err) { + /** + * If an opt out directive is present, log only instead of throwing and don't mark as + * containing a critical error. + */ + if (fn.node.body.type === 'BlockStatement') { + if (optOutDirectives.length > 0) { + logError(err, pass, fn.node.loc ?? null); + return null; + } + } hasCriticalError ||= isCriticalError(err); handleError(err, pass, fn.node.loc ?? null); return null; } + /** + * Always compile functions with opt in directives. + */ + if (optInDirectives.length > 0) { + return compiledFn; + } else if (pass.opts.compilationMode === 'annotation') { + /** + * No opt-in directive in annotation mode, so don't insert the compiled function. + */ + return null; + } + + /** + * Otherwise if 'use no forget/memo' is present, we still run the code through the compiler + * for validation but we don't mutate the babel AST. This allows us to flag if there is an + * unused 'use no forget/memo' directive. + */ + if (pass.opts.ignoreUseNoForget === false && optOutDirectives.length > 0) { + for (const directive of optOutDirectives) { + pass.opts.logger?.logEvent(pass.filename, { + kind: 'CompileSkip', + fnLoc: fn.node.body.loc ?? null, + reason: `Skipped due to '${directive.value.value}' directive.`, + loc: directive.loc ?? null, + }); + } + return null; + } + if (!pass.opts.noEmit && !hasCriticalError) { return compiledFn; } @@ -481,6 +531,16 @@ export function compileProgram( }); } + /** + * Do not modify source if there is a module scope level opt out directive. + */ + const moduleScopeOptOutDirectives = findDirectiveDisablingMemoization( + program.node.directives, + ); + if (moduleScopeOptOutDirectives.length > 0) { + return; + } + if (pass.opts.gating != null) { const error = checkFunctionReferencedBeforeDeclarationAtTopLevel( program, @@ -596,24 +656,6 @@ function shouldSkipCompilation( } } - // Top level "use no forget", skip this file entirely - const useNoForget = findDirectiveDisablingMemoization( - program.node.directives, - pass.opts, - ); - if (useNoForget != null) { - pass.opts.logger?.logEvent(pass.filename, { - kind: 'CompileError', - fnLoc: null, - detail: { - severity: ErrorSeverity.Todo, - reason: 'Skipped due to "use no forget" directive.', - loc: useNoForget.loc ?? null, - suggestions: null, - }, - }); - return true; - } const moduleName = pass.opts.runtimeModule ?? 'react/compiler-runtime'; if (hasMemoCacheFunctionImport(program, moduleName)) { return true; @@ -631,28 +673,8 @@ function getReactFunctionType( ): ReactFunctionType | null { const hookPattern = environment.hookPattern; if (fn.node.body.type === 'BlockStatement') { - // Opt-outs disable compilation regardless of mode - const useNoForget = findDirectiveDisablingMemoization( - fn.node.body.directives, - pass.opts, - ); - if (useNoForget != null) { - pass.opts.logger?.logEvent(pass.filename, { - kind: 'CompileError', - fnLoc: fn.node.body.loc ?? null, - detail: { - severity: ErrorSeverity.Todo, - reason: 'Skipped due to "use no forget" directive.', - loc: useNoForget.loc ?? null, - suggestions: null, - }, - }); - return null; - } - // Otherwise opt-ins enable compilation regardless of mode - if (findDirectiveEnablingMemoization(fn.node.body.directives) != null) { + if (findDirectiveEnablingMemoization(fn.node.body.directives).length > 0) return getComponentOrHookLike(fn, hookPattern) ?? 'Other'; - } } // Component and hook declarations are known components/hooks diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-forget-with-no-errors.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-forget-with-no-errors.expect.md new file mode 100644 index 0000000000000..20acbe0153135 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-forget-with-no-errors.expect.md @@ -0,0 +1,35 @@ + +## Input + +```javascript +function Component() { + 'use no forget'; + return
Hello World
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + isComponent: true, +}; + +``` + +## Code + +```javascript +function Component() { + "use no forget"; + return
Hello World
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + isComponent: true, +}; + +``` + +### Eval output +(kind: ok)
Hello World
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-forget-with-no-errors.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-forget-with-no-errors.js new file mode 100644 index 0000000000000..934487160d55c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-forget-with-no-errors.js @@ -0,0 +1,10 @@ +function Component() { + 'use no forget'; + return
Hello World
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + isComponent: true, +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/index.ts b/compiler/packages/babel-plugin-react-compiler/src/index.ts index f038246a4f1ee..aac65331a0ff2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/index.ts @@ -18,6 +18,7 @@ export { compileProgram, parsePluginOptions, run, + OPT_OUT_DIRECTIVES, type CompilerPipelineValue, type PluginOptions, } from './Entrypoint'; From 34edf3b68471e87d4a92f98a10f7c6c727c948f8 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 16 Aug 2024 17:39:51 -0400 Subject: [PATCH 4/5] [compiler] Surface unused opt out directives in eslint This PR updates the eslint plugin to report unused opt out directives. One of the downsides of the opt out directive is that it opts the component/hook out of compilation forever, even if the underlying issue was fixed in product code or fixed in the compiler. ghstack-source-id: 81deb5c11b7c57f07f6ab13266066cd73b2f3729 Pull Request resolved: https://github.com/facebook/react/pull/30721 --- .../__tests__/ReactCompilerRule-test.ts | 59 +++++++++++++++++++ .../src/rules/ReactCompilerRule.ts | 51 +++++++++++++++- 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts index c1aba9f4c8405..71be6b6622eb5 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts @@ -215,6 +215,65 @@ const tests: CompilerTestCases = { }, ], }, + { + name: "'use no forget' does not disable eslint rule", + code: normalizeIndent` + let count = 0; + function Component() { + 'use no forget'; + count = count + 1; + return
Hello world {count}
+ } + `, + errors: [ + { + message: + 'Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)', + }, + ], + }, + { + name: "Unused 'use no forget' directive is reported when no errors are present on components", + code: normalizeIndent` + function Component() { + 'use no forget'; + return
Hello world
+ } + `, + errors: [ + { + message: "Unused 'use no forget' directive", + suggestions: [ + { + output: + // yuck + '\nfunction Component() {\n \n return
Hello world
\n}\n', + }, + ], + }, + ], + }, + { + name: "Unused 'use no forget' directive is reported when no errors are present on non-components or hooks", + code: normalizeIndent` + function notacomponent() { + 'use no forget'; + return 1 + 1; + } + `, + errors: [ + { + message: "Unused 'use no forget' directive", + suggestions: [ + { + output: + // yuck + '\nfunction notacomponent() {\n \n return 1 + 1;\n}\n', + }, + ], + }, + ], + }, ], }; diff --git a/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts b/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts index 7b6525842c4d2..0a0956ebe1db0 100644 --- a/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts +++ b/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts @@ -15,10 +15,12 @@ import BabelPluginReactCompiler, { ErrorSeverity, parsePluginOptions, validateEnvironmentConfig, + OPT_OUT_DIRECTIVES, type PluginOptions, } from 'babel-plugin-react-compiler/src'; import {Logger} from 'babel-plugin-react-compiler/src/Entrypoint'; import type {Rule} from 'eslint'; +import {Statement} from 'estree'; import * as HermesParser from 'hermes-parser'; type CompilerErrorDetailWithLoc = Omit & { @@ -146,6 +148,7 @@ const rule: Rule.RuleModule = { userOpts['__unstable_donotuse_reportAllBailouts']; } + let shouldReportUnusedOptOutDirective = true; const options: PluginOptions = { ...parsePluginOptions(userOpts), ...COMPILER_OPTIONS, @@ -155,6 +158,7 @@ const rule: Rule.RuleModule = { logEvent: (filename, event): void => { userLogger?.logEvent(filename, event); if (event.kind === 'CompileError') { + shouldReportUnusedOptOutDirective = false; const detail = event.detail; const suggest = makeSuggestions(detail); if (__unstable_donotuse_reportAllBailouts && event.fnLoc != null) { @@ -272,7 +276,52 @@ const rule: Rule.RuleModule = { /* errors handled by injected logger */ } } - return {}; + + function reportUnusedOptOutDirective(stmt: Statement) { + if ( + stmt.type === 'ExpressionStatement' && + stmt.expression.type === 'Literal' && + typeof stmt.expression.value === 'string' && + OPT_OUT_DIRECTIVES.has(stmt.expression.value) && + stmt.loc != null + ) { + context.report({ + message: `Unused '${stmt.expression.value}' directive`, + loc: stmt.loc, + suggest: [ + { + desc: 'Remove the directive', + fix(fixer) { + return fixer.remove(stmt); + }, + }, + ], + }); + } + } + if (shouldReportUnusedOptOutDirective) { + return { + FunctionDeclaration(fnDecl) { + for (const stmt of fnDecl.body.body) { + reportUnusedOptOutDirective(stmt); + } + }, + ArrowFunctionExpression(fnExpr) { + if (fnExpr.body.type === 'BlockStatement') { + for (const stmt of fnExpr.body.body) { + reportUnusedOptOutDirective(stmt); + } + } + }, + FunctionExpression(fnExpr) { + for (const stmt of fnExpr.body.body) { + reportUnusedOptOutDirective(stmt); + } + }, + }; + } else { + return {}; + } }, }; From a58276cbc3a70ba99572eeb9c2f7b4a54ca44b1e Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 16 Aug 2024 17:39:51 -0400 Subject: [PATCH 5/5] [playground] Allow (Arrow)FunctionExpressions This was a pet peeve where our playground could only compile top level FunctionDeclarations. Just synthesize a fake identifier if it doesn't have one. ghstack-source-id: 882483c79ceebf382b69e37aed1f293efff9c5a7 Pull Request resolved: https://github.com/facebook/react/pull/30729 --- .../components/Editor/EditorImpl.tsx | 63 ++++++++++--------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index ebac65dc4b9f2..7b1214b4600c3 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -66,14 +66,14 @@ function parseFunctions( source: string, language: 'flow' | 'typescript', ): Array< - NodePath< - t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression - > + | NodePath + | NodePath + | NodePath > { const items: Array< - NodePath< - t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression - > + | NodePath + | NodePath + | NodePath > = []; try { const ast = parseInput(source, language); @@ -155,22 +155,33 @@ function isHookName(s: string): boolean { return /^use[A-Z0-9]/.test(s); } -function getReactFunctionType( - id: NodePath, -): ReactFunctionType { - if (id && id.node && id.isIdentifier()) { - if (isHookName(id.node.name)) { +function getReactFunctionType(id: t.Identifier | null): ReactFunctionType { + if (id != null) { + if (isHookName(id.name)) { return 'Hook'; } const isPascalCaseNameSpace = /^[A-Z].*/; - if (isPascalCaseNameSpace.test(id.node.name)) { + if (isPascalCaseNameSpace.test(id.name)) { return 'Component'; } } return 'Other'; } +function getFunctionIdentifier( + fn: + | NodePath + | NodePath + | NodePath, +): t.Identifier | null { + if (fn.isArrowFunctionExpression()) { + return null; + } + const id = fn.get('id'); + return Array.isArray(id) === false && id.isIdentifier() ? id.node : null; +} + function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { const results = new Map(); const error = new CompilerError(); @@ -188,27 +199,21 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { } else { language = 'typescript'; } + let count = 0; + const withIdentifier = (id: t.Identifier | null): t.Identifier => { + if (id != null && id.name != null) { + return id; + } else { + return t.identifier(`anonymous_${count++}`); + } + }; try { // Extract the first line to quickly check for custom test directives const pragma = source.substring(0, source.indexOf('\n')); const config = parseConfigPragma(pragma); for (const fn of parseFunctions(source, language)) { - if (!fn.isFunctionDeclaration()) { - error.pushErrorDetail( - new CompilerErrorDetail({ - reason: `Unexpected function type ${fn.node.type}`, - description: - 'Playground only supports parsing function declarations', - severity: ErrorSeverity.Todo, - loc: fn.node.loc ?? null, - suggestions: null, - }), - ); - continue; - } - - const id = fn.get('id'); + const id = withIdentifier(getFunctionIdentifier(fn)); for (const result of run( fn, { @@ -221,7 +226,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { null, null, )) { - const fnName = fn.node.id?.name ?? null; + const fnName = id.name; switch (result.kind) { case 'ast': { upsert({ @@ -230,7 +235,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { name: result.name, value: { type: 'FunctionDeclaration', - id: result.value.id, + id, async: result.value.async, generator: result.value.generator, body: result.value.body,