Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fax] Support nesting in existing RSC renderers #30736

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
319 changes: 319 additions & 0 deletions packages/react-markup/src/__tests__/ReactMarkupAndFlight-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,319 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
* @jest-environment node
*/

'use strict';

if (typeof Blob === 'undefined') {
global.Blob = require('buffer').Blob;
}
if (typeof File === 'undefined' || typeof FormData === 'undefined') {
global.File = require('undici').File;
global.FormData = require('undici').FormData;
}

let act;
let React;
let ReactServer;
let ReactMarkup;
let ReactNoop;
let ReactNoopFlightServer;
let ReactNoopFlightClient;

function normalizeCodeLocInfo(str) {
return (
str &&
String(str).replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
return '\n in ' + name + ' (at **)';
})
);
}

if (!__EXPERIMENTAL__) {
it('should not be built in stable', () => {
try {
require('react-markup');
} catch (x) {
return;
}
throw new Error('Expected react-markup not to exist in stable.');
});
} else {
describe('ReactMarkupAndFlight', () => {
beforeEach(() => {
jest.resetModules();
jest.mock('react', () => require('react/react.react-server'));
ReactServer = require('react');
ReactNoopFlightServer = require('react-noop-renderer/flight-server');
// This stores the state so we need to preserve it
const flightModules = require('react-noop-renderer/flight-modules');
if (__EXPERIMENTAL__) {
jest.resetModules();
jest.mock('react', () => ReactServer);
jest.mock('react-markup', () =>
require('react-markup/react-markup.react-server'),
);
ReactMarkup = require('react-markup');
}
jest.resetModules();
__unmockReact();
jest.mock('react-noop-renderer/flight-modules', () => flightModules);
React = require('react');
ReactNoop = require('react-noop-renderer');
ReactNoopFlightClient = require('react-noop-renderer/flight-client');
act = require('internal-test-utils').act;
});

afterEach(() => {
jest.restoreAllMocks();
});

it('supports using react-markup', async () => {
async function Preview() {
const html =
await ReactMarkup.experimental_renderToHTML('Hello, Dave!');

return <pre>{html}</pre>;
}

const model = <Preview />;
const transport = ReactNoopFlightServer.render(model);

await act(async () => {
// So it throws here with "Cannot read properties of null (reading 'length')"
ReactNoop.render(await ReactNoopFlightClient.read(transport));
});

expect(ReactNoop).toMatchRenderedOutput(<pre>Hello, Dave!</pre>);
});

it('has a cache if the first renderer is used standalone', async () => {
let n = 0;
const uncachedFunction = jest.fn(() => {
return n++;
});
const random = ReactServer.cache(uncachedFunction);

function Random() {
return random();
}

function App() {
return (
<>
<p>
RSC A_1: <Random />
</p>
<p>
RSC A_2: <Random />
</p>
</>
);
}

const model = <App />;
const transport = ReactNoopFlightServer.render(model);

await act(async () => {
ReactNoop.render(await ReactNoopFlightClient.read(transport));
});

expect(ReactNoop).toMatchRenderedOutput(
<>
<p>RSC A_1: 0</p>
<p>RSC A_2: 0</p>
</>,
);
expect(uncachedFunction).toHaveBeenCalledTimes(1);
});

it('has a cache if the second renderer is used standalone', async () => {
let n = 0;
const uncachedFunction = jest.fn(() => {
return n++;
});
const random = ReactServer.cache(uncachedFunction);

function Random() {
return random();
}

function App() {
return (
<>
<p>
RSC B_1: <Random />
</p>
<p>
RSC B_2: <Random />
</p>
</>
);
}

const html = await ReactMarkup.experimental_renderToHTML(
ReactServer.createElement(App),
);

expect(html).toEqual('<p>RSC B_1: 0</p><p>RSC B_2: 0</p>');
expect(uncachedFunction).toHaveBeenCalledTimes(1);
});

it('shares cache between RSC renderers', async () => {
let n = 0;
const uncachedFunction = jest.fn(() => {
return n++;
});
const random = ReactServer.cache(uncachedFunction);

function Random() {
return random();
}

async function Preview() {
const html = await ReactMarkup.experimental_renderToHTML(<Random />);

return (
<>
<p>
RSC A: <Random />
</p>
<p>RSC B: {html}</p>
</>
);
}

function App() {
return (
<>
<Preview />
<Preview />
</>
);
}

const model = <App />;
const transport = ReactNoopFlightServer.render(model);

await act(async () => {
ReactNoop.render(await ReactNoopFlightClient.read(transport));
});

expect(ReactNoop).toMatchRenderedOutput(
<>
<p>RSC A: 0</p>
<p>RSC B: 0</p>
<p>RSC A: 0</p>
<p>RSC B: 0</p>
</>,
);
expect(uncachedFunction).toHaveBeenCalledTimes(1);
});

it('shows correct stacks in nested RSC renderers', async () => {
const thrownError = new Error('hi');

const caughtNestedRendererErrors = [];
const ownerStacksDuringParentRendererThrow = [];
const ownerStacksDuringNestedRendererThrow = [];
function Throw() {
if (gate(flags => flags.enableOwnerStacks)) {
const stack = ReactServer.captureOwnerStack();
ownerStacksDuringNestedRendererThrow.push(
normalizeCodeLocInfo(stack),
);
}
throw thrownError;
}

function Indirection() {
return ReactServer.createElement(Throw);
}

function App() {
return ReactServer.createElement(Indirection);
}

async function Preview() {
try {
await ReactMarkup.experimental_renderToHTML(
ReactServer.createElement(App),
{
onError: (error, errorInfo) => {
caughtNestedRendererErrors.push({
error: error,
parentStack: errorInfo.componentStack,
ownerStack: gate(flags => flags.enableOwnerStacks)
? ReactServer.captureOwnerStack()
: null,
});
},
},
);
} catch (error) {
let stack = '';
if (gate(flags => flags.enableOwnerStacks)) {
stack = ReactServer.captureOwnerStack();
ownerStacksDuringParentRendererThrow.push(
normalizeCodeLocInfo(stack),
);
}

return 'did error';
}
}

function PreviewApp() {
return ReactServer.createElement(Preview);
}

const model = ReactServer.createElement(PreviewApp);
const transport = ReactNoopFlightServer.render(model);

await act(async () => {
ReactNoop.render(await ReactNoopFlightClient.read(transport));
});

expect(caughtNestedRendererErrors).toEqual([
{
error: thrownError,
ownerStack:
__DEV__ && gate(flags => flags.enableOwnerStacks)
? // TODO: Shouldn't this read the same as the one we got during render?
''
: null,
// TODO: Shouldn't a parent stack exist?
parentStack: undefined,
},
]);
expect(ownerStacksDuringParentRendererThrow).toEqual(
gate(flags => flags.enableOwnerStacks)
? [
__DEV__
? // TODO: Should have an owner stack
''
: null,
]
: [],
);
expect(ownerStacksDuringNestedRendererThrow).toEqual(
gate(flags => flags.enableOwnerStacks)
? [
__DEV__
? '\n' +
//
' in Indirection (at **)\n' +
' in App (at **)'
: null,
]
: [],
);
});
});
}
13 changes: 10 additions & 3 deletions packages/react-reconciler/src/ReactFiberAsyncDispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,27 @@ import {disableStringRefs} from 'shared/ReactFeatureFlags';

import {current as currentOwner} from './ReactCurrentFiber';

function getActiveCache(): Map<Function, mixed> {
const cache: Cache = readContext(CacheContext);

return cache.data;
}

function getCacheForType<T>(resourceType: () => T): T {
if (!enableCache) {
throw new Error('Not implemented.');
}
const cache: Cache = readContext(CacheContext);
let cacheForType: T | void = (cache.data.get(resourceType): any);
const cache = getActiveCache();
let cacheForType: T | void = (cache.get(resourceType): any);
if (cacheForType === undefined) {
cacheForType = resourceType();
cache.data.set(resourceType, cacheForType);
cache.set(resourceType, cacheForType);
}
return cacheForType;
}

export const DefaultAsyncDispatcher: AsyncDispatcher = ({
getActiveCache,
getCacheForType,
}: any);

Expand Down
1 change: 1 addition & 0 deletions packages/react-reconciler/src/ReactInternalTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ export type Dispatcher = {
};

export type AsyncDispatcher = {
getActiveCache: () => Map<Function, mixed> | null,
getCacheForType: <T>(resourceType: () => T) => T,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for both of these. If we're saying that the implementation detail is a Map<Function> and that is exposed through getActiveCache(), there's not that much getCacheForType can configure and it's unnecessary indirection. getCacheForType can just be a utility function that calls getActiveCache. (Or we can get rid of getCacheForType completely and just have the callers call getActiveCache.) No need to be on the dispatcher anymore.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To preempt "I'll do it in a follow up". Since we know that these dispatchers are messed with by people and we break them we don't want to change this too many times. Even the async cache ones (cc Janka).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like this f140f36 (#30736)?

Whatever resolves the cache conflict needs to know the previous dispatcher so it made sense to me to just do the resolution inside getActiveCache. I don't know how else to do it if we want getActiveCache to return the actual Map (or null) and resolve conflicts in a utility since that utility needs to know the current and previous dispatcher.

// DEV-only (or !disableStringRefs)
getOwner: () => null | Fiber | ReactComponentInfo | ComponentStackNode,
Expand Down
11 changes: 2 additions & 9 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ const {
TaintRegistryPendingRequests,
} = ReactSharedInternals;

ReactSharedInternals.A = DefaultAsyncDispatcher;

function throwTaintViolation(message: string) {
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(message);
Expand Down Expand Up @@ -448,15 +450,6 @@ function RequestInstance(
onAllReady: () => void,
onFatalError: (error: mixed) => void,
) {
if (
ReactSharedInternals.A !== null &&
ReactSharedInternals.A !== DefaultAsyncDispatcher
) {
throw new Error(
'Currently React only supports one RSC renderer at a time.',
);
}
ReactSharedInternals.A = DefaultAsyncDispatcher;
if (__DEV__) {
// Unlike Fizz or Fiber, we don't reset this and just keep it on permanently.
// This lets it act more like the AsyncDispatcher so that we can get the
Expand Down
Loading
Loading