Skip to content

Commit

Permalink
Don't dedupe Elements if they're in a non-default Context
Browse files Browse the repository at this point in the history
If an element gets wrapped in a different server component then that has
a different keyPath context and the element might end up with a different
key.

So we don't use the deduping mechanism if we're already inside a
Server Component parent with a key or otherwise. Only the simple case
gets deduped.

The props of a client element are still deduped though if they're the
same instance.
  • Loading branch information
sebmarkbage committed Jan 27, 2024
1 parent cdeec64 commit e80cdb0
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,37 @@ describe('ReactFlightDOMEdge', () => {
const stream = ReactServerDOMServer.renderToReadableStream(children);
const [stream1, stream2] = passThrough(stream).tee();

const serializedContent = await readResult(stream1);

expect(serializedContent.length).toBeLessThan(400);
expect(timesRendered).toBeLessThan(5);

const result = await ReactServerDOMClient.createFromReadableStream(
stream2,
{
ssrManifest: {
moduleMap: null,
moduleLoading: null,
},
},
);
// Should still match the result when parsed
expect(result).toEqual(resolvedChildren);
});

it('should execute repeated host components only once', async () => {
const div = <div>this is a long return value</div>;
let timesRendered = 0;
function ServerComponent() {
timesRendered++;
return div;
}
const element = <ServerComponent />;
const children = new Array(30).fill(element);
const resolvedChildren = new Array(30).fill(div);
const stream = ReactServerDOMServer.renderToReadableStream(children);
const [stream1, stream2] = passThrough(stream).tee();

const serializedContent = await readResult(stream1);
expect(serializedContent.length).toBeLessThan(400);
expect(timesRendered).toBeLessThan(5);
Expand Down
32 changes: 24 additions & 8 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -787,10 +787,6 @@ function createTask(
abortSet: Set<Task>,
): Task {
const id = request.nextChunkId++;
if (typeof model === 'object' && model !== null) {
// Register this model as having the ID we're about to write.
request.writtenObjects.set(model, id);
}
const task: Task = {
id,
status: PENDING,
Expand Down Expand Up @@ -988,7 +984,17 @@ function serializeClientReference(
}
}

function outlineModel(request: Request, value: ReactClientValue): number {
function outlineModel(
request: Request,
value:
| ClientReference<any>
| ServerReference<any>
| Iterable<ReactClientValue>
| Array<ReactClientValue>
| Map<ReactClientValue, ReactClientValue>
| Set<ReactClientValue>
| ReactClientObject,
): number {
request.pendingChunks++;
const newTask = createTask(
request,
Expand All @@ -998,6 +1004,7 @@ function outlineModel(request: Request, value: ReactClientValue): number {
rootContextSnapshot, // Therefore we don't pass any contextual information along.
request.abortableTasks,
);
request.writtenObjects.set(value, newTask.id);
retryTask(request, newTask);
return newTask.id;
}
Expand Down Expand Up @@ -1251,9 +1258,18 @@ function renderModelDestructive(
const writtenObjects = request.writtenObjects;
const existingId = writtenObjects.get(value);
if (existingId !== undefined) {
if (existingId === -1) {
if (
enableServerComponentKeys &&
(task.keyPath !== null ||
task.implicitSlot ||
task.context !== rootContextSnapshot)
) {
// If we're in some kind of context we can't reuse the result of this render or
// previous renders of this element. We only reuse elements if they're not wrapped
// by another Server Component.
} else if (existingId === -1) {
// Seen but not yet outlined.
const newId = outlineModel(request, value);
const newId = outlineModel(request, (value: any));
return serializeByValueID(newId);
} else if (modelRoot === value) {
// This is the ID we're currently emitting so we need to write it
Expand Down Expand Up @@ -1360,7 +1376,7 @@ function renderModelDestructive(
if (existingId !== undefined) {
if (existingId === -1) {
// Seen but not yet outlined.
const newId = outlineModel(request, value);
const newId = outlineModel(request, (value: any));
return serializeByValueID(newId);
} else if (modelRoot === value) {
// This is the ID we're currently emitting so we need to write it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@

declare var $$$config: any;

interface Reference {}

export opaque type ClientManifest = mixed;
export opaque type ClientReference<T> = mixed; // eslint-disable-line no-unused-vars
export opaque type ServerReference<T> = mixed; // eslint-disable-line no-unused-vars
export opaque type ClientReference<T>: Reference = Reference; // eslint-disable-line no-unused-vars
export opaque type ServerReference<T>: Reference = Reference; // eslint-disable-line no-unused-vars
export opaque type ClientReferenceMetadata: any = mixed;
export opaque type ServerReferenceId: any = mixed;
export opaque type ClientReferenceKey: any = mixed;
Expand Down

0 comments on commit e80cdb0

Please sign in to comment.