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

[Flight] Encode Symbols as special rows that can be referenced by models … #20171

Merged
merged 3 commits into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,13 @@ function createErrorChunk(response: Response, error: Error): ErroredChunk {
return new Chunk(ERRORED, error, response);
}

function createInitializedChunk<T>(
response: Response,
value: T,
): InitializedChunk<T> {
return new Chunk(INITIALIZED, value, response);
}

function wakeChunk(listeners: null | Array<() => mixed>) {
if (listeners !== null) {
for (let i = 0; i < listeners.length; i++) {
Expand Down Expand Up @@ -373,6 +380,17 @@ export function resolveModule(
}
}

export function resolveSymbol(
response: Response,
id: number,
name: string,
): void {
const chunks = response._chunks;
// We assume that we'll always emit the symbol before anything references it
// to save a few bytes.
chunks.set(id, createInitializedChunk(response, Symbol.for(name)));
}

export function resolveError(
response: Response,
id: number,
Expand Down
27 changes: 15 additions & 12 deletions packages/react-client/src/ReactFlightClientStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {Response} from './ReactFlightClientHostConfigStream';
import {
resolveModule,
resolveModel,
resolveSymbol,
resolveError,
createResponse as createResponseBase,
parseModelString,
Expand All @@ -32,26 +33,28 @@ function processFullRow(response: Response, row: string): void {
return;
}
const tag = row[0];
// When tags that are not text are added, check them here before
// parsing the row as text.
// switch (tag) {
// }
const colon = row.indexOf(':', 1);
const id = parseInt(row.substring(1, colon), 16);
const text = row.substring(colon + 1);
switch (tag) {
case 'J': {
const colon = row.indexOf(':', 1);
const id = parseInt(row.substring(1, colon), 16);
const json = row.substring(colon + 1);
resolveModel(response, id, json);
resolveModel(response, id, text);
return;
}
case 'M': {
const colon = row.indexOf(':', 1);
const id = parseInt(row.substring(1, colon), 16);
const json = row.substring(colon + 1);
resolveModule(response, id, json);
resolveModule(response, id, text);
return;
}
case 'S': {
resolveSymbol(response, id, JSON.parse(text));
return;
}
case 'E': {
const colon = row.indexOf(':', 1);
const id = parseInt(row.substring(1, colon), 16);
const json = row.substring(colon + 1);
const errorInfo = JSON.parse(json);
const errorInfo = JSON.parse(text);
resolveError(response, id, errorInfo.message, errorInfo.stack);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ describe('ReactFlight', () => {
<ErrorBoundary expectedMessage="Functions cannot be passed directly to client components because they're not serializable.">
<Client transport={fn} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Symbol values (foo) cannot be passed to client components.">
<ErrorBoundary expectedMessage="Only global symbols received from Symbol.for(...) can be passed to client components.">
<Client transport={symbol} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Refs cannot be used in server components, nor passed to client components.">
Expand Down
90 changes: 55 additions & 35 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,20 @@ import {
close,
processModelChunk,
processModuleChunk,
processSymbolChunk,
processErrorChunk,
resolveModuleMetaData,
isModuleReference,
} from './ReactFlightServerConfig';

import {
REACT_ELEMENT_TYPE,
REACT_DEBUG_TRACING_MODE_TYPE,
REACT_FORWARD_REF_TYPE,
REACT_FRAGMENT_TYPE,
REACT_LAZY_TYPE,
REACT_LEGACY_HIDDEN_TYPE,
REACT_MEMO_TYPE,
REACT_OFFSCREEN_TYPE,
REACT_PROFILER_TYPE,
REACT_SCOPE_TYPE,
REACT_STRICT_MODE_TYPE,
REACT_SUSPENSE_TYPE,
REACT_SUSPENSE_LIST_TYPE,
} from 'shared/ReactSymbols';

import * as React from 'react';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';

Expand Down Expand Up @@ -86,6 +78,7 @@ export type Request = {
completedModuleChunks: Array<Chunk>,
completedJSONChunks: Array<Chunk>,
completedErrorChunks: Array<Chunk>,
writtenSymbols: Map<Symbol, number>,
flowing: boolean,
toJSON: (key: string, value: ReactModel) => ReactJSONValue,
};
Expand All @@ -107,6 +100,7 @@ export function createRequest(
completedModuleChunks: [],
completedJSONChunks: [],
completedErrorChunks: [],
writtenSymbols: new Map(),
flowing: false,
toJSON: function(key: string, value: ReactModel): ReactJSONValue {
return resolveModelToJSON(request, this, key, value);
Expand All @@ -118,10 +112,13 @@ export function createRequest(
return request;
}

function attemptResolveElement(element: React$Element<any>): ReactModel {
const type = element.type;
const props = element.props;
if (element.ref !== null && element.ref !== undefined) {
function attemptResolveElement(
type: any,
key: null | React$Key,
ref: mixed,
props: any,
): ReactModel {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain what you fixed here? I don't get it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a separate commit 29b9646

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah but I don't understand what it was fixing

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see the REACT_MEMO_TYPE case. Was it losing element.key and element.ref earlier because you did createElement(type, element.props)?

Copy link
Collaborator Author

@sebmarkbage sebmarkbage Nov 6, 2020

Choose a reason for hiding this comment

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

You did. :) But yea and we might remove the export at some point too. Regardless, it's just less efficient to create an unnecessary element.

if (ref !== null && ref !== undefined) {
// When the ref moves to the regular props object this will implicitly
// throw for functions. We could probably relax it to a DEV warning for other
// cases.
Expand All @@ -135,34 +132,30 @@ function attemptResolveElement(element: React$Element<any>): ReactModel {
return type(props);
} else if (typeof type === 'string') {
// This is a host element. E.g. HTML.
return [REACT_ELEMENT_TYPE, type, element.key, element.props];
} else if (
type === REACT_FRAGMENT_TYPE ||
type === REACT_STRICT_MODE_TYPE ||
type === REACT_PROFILER_TYPE ||
type === REACT_SCOPE_TYPE ||
type === REACT_DEBUG_TRACING_MODE_TYPE ||
type === REACT_LEGACY_HIDDEN_TYPE ||
type === REACT_OFFSCREEN_TYPE ||
// TODO: These are temporary shims
// and we'll want a different behavior.
type === REACT_SUSPENSE_TYPE ||
type === REACT_SUSPENSE_LIST_TYPE
) {
return element.props.children;
return [REACT_ELEMENT_TYPE, type, key, props];
} else if (typeof type === 'symbol') {
if (type === REACT_FRAGMENT_TYPE) {
// For key-less fragments, we add a small optimization to avoid serializing
// it as a wrapper.
// TODO: If a key is specified, we should propagate its key to any children.
// Same as if a server component has a key.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This actually reminded me.. What's the plan for different types? Do those also get encoded somehow when we get to encoding keys? Or does the type change trick not work for Server Components?

Copy link
Collaborator Author

@sebmarkbage sebmarkbage Nov 6, 2020

Choose a reason for hiding this comment

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

Yea, it doesn't work for server components but that was already the plan for function components too as part of the "inlining optimization" or "call through". At least stateless ones and only if they don't have an explicit key. The reason it needs to kill state is because the state doesn't "line up" with a different type. A stateless doesn't need to.

It shouldn't be relied upon. If you need it, it should be a different key.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh right. So it doesn't matter until you run into a Client one, at which point you have the types on the client anyway.

I guess I still find this a bit counter-intuitive in terms of guarantees. It allows for some fun patterns though. Like SCs could use recursion and then have a terminal CC, and it could still match up on refetch despite different SC depth?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. You could also just call a recursive function. I guess you could use it as a resuming hack.

return props.children;
}
// This might be a built-in React component. We'll let the client decide.
// Any built-in works as long as its props are serializable.
return [REACT_ELEMENT_TYPE, type, key, props];
} else if (type != null && typeof type === 'object') {
if (isModuleReference(type)) {
// This is a reference to a client component.
return [REACT_ELEMENT_TYPE, type, element.key, element.props];
return [REACT_ELEMENT_TYPE, type, key, props];
}
switch (type.$$typeof) {
case REACT_FORWARD_REF_TYPE: {
const render = type.render;
return render(props, undefined);
}
case REACT_MEMO_TYPE: {
const nextChildren = React.createElement(type.type, element.props);
return attemptResolveElement(nextChildren);
return attemptResolveElement(type.type, key, ref, props);
}
}
}
Expand Down Expand Up @@ -399,7 +392,12 @@ export function resolveModelToJSON(
const element: React$Element<any> = (value: any);
try {
// Attempt to render the server component.
value = attemptResolveElement(element);
value = attemptResolveElement(
element.type,
element.key,
element.ref,
element.props,
);
} catch (x) {
if (typeof x === 'object' && x !== null && typeof x.then === 'function') {
// Something suspended, we'll need to create a new segment and resolve it later.
Expand Down Expand Up @@ -526,14 +524,26 @@ export function resolveModelToJSON(
}

if (typeof value === 'symbol') {
const writtenSymbols = request.writtenSymbols;
const existingId = writtenSymbols.get(value);
if (existingId !== undefined) {
return serializeByValueID(existingId);
}
const name = value.description;
Copy link
Collaborator

Choose a reason for hiding this comment

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

😲 TIL

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IIRC, it was actually added late or even later version of the spec.

invariant(
false,
'Symbol values (%s) cannot be passed to client components. ' +
Symbol.for(name) === value,
'Only global symbols received from Symbol.for(...) can be passed to client components. ' +
'The symbol Symbol.for(%s) cannot be found among global symbols. ' +
'Remove %s from this object, or avoid the entire object: %s',
value.description,
describeKeyForErrorMessage(key),
describeObjectForErrorMessage(parent),
);
request.pendingChunks++;
const symbolId = request.nextChunkId++;
emitSymbolChunk(request, symbolId, name);
writtenSymbols.set(value, symbolId);
return serializeByValueID(symbolId);
}

// $FlowFixMe: bigint isn't added to Flow yet.
Expand Down Expand Up @@ -588,6 +598,11 @@ function emitModuleChunk(
request.completedModuleChunks.push(processedChunk);
}

function emitSymbolChunk(request: Request, id: number, name: string): void {
const processedChunk = processSymbolChunk(request, id, name);
request.completedModuleChunks.push(processedChunk);
}

function retrySegment(request: Request, segment: Segment): void {
const query = segment.query;
let value;
Expand All @@ -604,7 +619,12 @@ function retrySegment(request: Request, segment: Segment): void {
// Doing this here lets us reuse this same segment if the next component
// also suspends.
segment.query = () => value;
value = attemptResolveElement(element);
value = attemptResolveElement(
element.type,
element.key,
element.ref,
element.props,
);
}
const processedChunk = processModelChunk(request, segment.id, value);
request.completedJSONChunks.push(processedChunk);
Expand Down
10 changes: 10 additions & 0 deletions packages/react-server/src/ReactFlightServerConfigStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ export function processModuleChunk(
return convertStringToBuffer(row);
}

export function processSymbolChunk(
request: Request,
id: number,
name: string,
): Chunk {
const json = stringify(name);
const row = serializeRowHeader('S', id) + json + '\n';
return convertStringToBuffer(row);
}

export {
scheduleWork,
flushBuffered,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
createResponse,
resolveModel,
resolveModule,
resolveSymbol,
resolveError,
close,
} from 'react-client/src/ReactFlightClient';
Expand All @@ -26,6 +27,9 @@ export function resolveRow(response: Response, chunk: RowEncoding): void {
resolveModel(response, chunk[1], chunk[2]);
} else if (chunk[0] === 'M') {
resolveModule(response, chunk[1], chunk[2]);
} else if (chunk[0] === 'S') {
// $FlowFixMe: Flow doesn't support disjoint unions on tuples.
resolveSymbol(response, chunk[1], chunk[2]);
} else {
// $FlowFixMe: Flow doesn't support disjoint unions on tuples.
resolveError(response, chunk[1], chunk[2].message, chunk[2].stack);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type JSONValue =
export type RowEncoding =
| ['J', number, JSONValue]
| ['M', number, ModuleMetaData]
| ['S', number, string]
| [
'E',
number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@ export function processModuleChunk(
return ['M', id, moduleMetaData];
}

export function processSymbolChunk(
request: Request,
id: number,
name: string,
): Chunk {
return ['S', id, name];
}

export function scheduleWork(callback: () => void) {
callback();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,14 @@ describe('ReactFlightDOMRelay', () => {
foo: {
bar: (
<div>
{'Fragment child'}
{'Profiler child'}
{'StrictMode child'}
{'Suspense child'}
{['SuspenseList row 1', 'SuspenseList row 2']}
Fragment child
<Profiler>Profiler child</Profiler>
<StrictMode>StrictMode child</StrictMode>
<Suspense fallback="Loading...">Suspense child</Suspense>
<SuspenseList fallback="Loading...">
{'SuspenseList row 1'}
{'SuspenseList row 2'}
</SuspenseList>
<div>Hello world</div>
</div>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,6 @@ describe('ReactFlightDOM', () => {
);
}

function Placeholder({children, fallback}) {
return <Suspense fallback={fallback}>{children}</Suspense>;
}

// Model
function Text({children}) {
return children;
Expand Down Expand Up @@ -347,22 +343,21 @@ describe('ReactFlightDOM', () => {
}

const MyErrorBoundaryClient = moduleReference(MyErrorBoundary);
const PlaceholderClient = moduleReference(Placeholder);

function ProfileContent() {
return (
<>
<ProfileDetails avatar={<Text>:avatar:</Text>} />
<PlaceholderClient fallback={<p>(loading sidebar)</p>}>
<Suspense fallback={<p>(loading sidebar)</p>}>
<ProfileSidebar friends={<Friends>:friends:</Friends>} />
</PlaceholderClient>
<PlaceholderClient fallback={<p>(loading posts)</p>}>
</Suspense>
<Suspense fallback={<p>(loading posts)</p>}>
<ProfilePosts posts={<Posts>:posts:</Posts>} />
</PlaceholderClient>
</Suspense>
<MyErrorBoundaryClient>
<PlaceholderClient fallback={<p>(loading games)</p>}>
<Suspense fallback={<p>(loading games)</p>}>
<ProfileGames games={<Games>:games:</Games>} />
</PlaceholderClient>
</Suspense>
</MyErrorBoundaryClient>
</>
);
Expand Down
Loading