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

[RSC @ Meta] Simplify implementation of isClientReference, getClientReferenceKey, resolveClientReferenceMetadata #27839

Merged
merged 6 commits into from
Dec 19, 2023
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
9 changes: 6 additions & 3 deletions packages/react-server-dom-fb/src/ReactFlightDOMServerFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import type {
Chunk,
PrecomputedChunk,
} from 'react-server/src/ReactServerStreamConfig';
import type {ClientManifest} from './ReactFlightReferencesFB';

import {setCheckIsClientReference} from './ReactFlightReferencesFB';

import {
createRequest,
Expand All @@ -28,6 +29,7 @@ export {
registerServerReference,
getRequestedClientReferencesKeys,
clearRequestedClientReferencesKeysSet,
setCheckIsClientReference,
} from './ReactFlightReferencesFB';

type Options = {
Expand All @@ -37,7 +39,6 @@ type Options = {
function renderToDestination(
destination: Destination,
model: ReactClientValue,
bundlerConfig: ClientManifest,
options?: Options,
): void {
if (!configured) {
Expand All @@ -47,7 +48,7 @@ function renderToDestination(
}
const request = createRequest(
model,
bundlerConfig,
null,
options ? options.onError : undefined,
);
startWork(request);
Expand All @@ -56,12 +57,14 @@ function renderToDestination(

type Config = {
byteLength: (chunk: Chunk | PrecomputedChunk) => number,
isClientReference: (reference: mixed) => boolean,
};

let configured = false;

function setConfig(config: Config): void {
setByteLengthOfChunkImplementation(config.byteLength);
setCheckIsClientReference(config.isClientReference);
configured = true;
}

Expand Down
56 changes: 24 additions & 32 deletions packages/react-server-dom-fb/src/ReactFlightReferencesFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@
* @flow
*/

export opaque type ClientManifest = mixed;
export type ClientManifest = null;

// eslint-disable-next-line no-unused-vars
export type ServerReference<T> = string;

// eslint-disable-next-line no-unused-vars
Comment on lines 12 to 15

Choose a reason for hiding this comment

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

are the // eslint-disable-next-line no-unused-vars actually needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.

export type ClientReference<T> = string;
export type ClientReference<T> = {
getModuleId(): ClientReferenceKey,
};

const registeredClientReferences = new Map<mixed, ClientReferenceMetadata>();
const requestedClientReferencesKeys = new Set<ClientReferenceKey>();

export type ClientReferenceKey = string;
Expand All @@ -26,54 +27,45 @@ export type ClientReferenceMetadata = {

export type ServerReferenceId = string;

let checkIsClientReference: (clientReference: mixed) => boolean;

export function setCheckIsClientReference(
impl: (clientReference: mixed) => boolean,
): void {
checkIsClientReference = impl;
}

export function registerClientReference<T>(
clientReference: ClientReference<T>,
moduleId: ClientReferenceKey,
): ClientReference<T> {
const exportName = 'default'; // Currently, we only support modules with `default` export
registeredClientReferences.set(clientReference, {
moduleId,
exportName,
});

return clientReference;
}
): void {}

export function isClientReference<T>(reference: T): boolean {
return registeredClientReferences.has(reference);
export function isClientReference(reference: mixed): boolean {
if (checkIsClientReference == null) {
throw new Error('Expected implementation for checkIsClientReference.');
}
return checkIsClientReference(reference);
}

export function getClientReferenceKey<T>(
clientReference: ClientReference<T>,
): ClientReferenceKey {
const reference = registeredClientReferences.get(clientReference);
if (reference != null) {
requestedClientReferencesKeys.add(reference.moduleId);
return reference.moduleId;
}
const moduleId = clientReference.getModuleId();
requestedClientReferencesKeys.add(moduleId);

throw new Error(
'Expected client reference ' + clientReference + ' to be registered.',
);
return clientReference.getModuleId();
}

export function resolveClientReferenceMetadata<T>(
config: ClientManifest,
clientReference: ClientReference<T>,
): ClientReferenceMetadata {
const metadata = registeredClientReferences.get(clientReference);
if (metadata != null) {
return metadata;
}

throw new Error(
'Expected client reference ' + clientReference + ' to be registered.',
);
return {moduleId: clientReference.getModuleId(), exportName: 'default'};
}

export function registerServerReference<T>(
serverReference: ServerReference<T>,
exportName: string,
id: string,
exportName: null | string,
): ServerReference<T> {
throw new Error('registerServerReference: Not Implemented.');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ let ReactDOMClient;
let ReactServerDOMServer;
let ReactServerDOMClient;
let Suspense;
let registerClientReference;

class Destination {
#buffer = '';
Expand Down Expand Up @@ -57,14 +56,22 @@ class Destination {
onError() {}
}

class ClientReferenceImpl {
constructor(moduleId) {
this.moduleId = moduleId;
}

getModuleId() {
return this.moduleId;
}
}

describe('ReactFlightDOM for FB', () => {
beforeEach(() => {
// For this first reset we are going to load the dom-node version of react-server-dom-turbopack/server
// This can be thought of as essentially being the React Server Components scope with react-server
// condition
jest.resetModules();
registerClientReference =
require('../ReactFlightReferencesFB').registerClientReference;

jest.mock('react', () => require('react/src/ReactSharedSubsetFB'));

Expand All @@ -78,8 +85,7 @@ describe('ReactFlightDOM for FB', () => {
});

clientExports = value => {
registerClientReference(value, value.name);
return value;
return new ClientReferenceImpl(value.name);
};

moduleMap = {
Expand All @@ -91,6 +97,7 @@ describe('ReactFlightDOM for FB', () => {
ReactServerDOMServer = require('../ReactFlightDOMServerFB');
ReactServerDOMServer.setConfig({
byteLength: str => Buffer.byteLength(str),
isClientReference: reference => reference instanceof ClientReferenceImpl,
});

// This reset is to load modules for the SSR/Browser scope.
Expand Down