Skip to content

Commit

Permalink
add new IDs for each each server renderer instance and prefixes to di…
Browse files Browse the repository at this point in the history
…stinguish between each server render (#18576)

There is a worry that `useOpaqueIdentifier` might run out of unique IDs if running for long enough. This PR moves the unique ID counter so it's generated per server renderer object instead. For people who render different subtrees, this PR adds a prefix option to `renderToString`, `renderToStaticMarkup`, `renderToNodeStream`, and `renderToStaticNodeStream` so identifiers can be differentiated for each individual subtree.
  • Loading branch information
lunaruan authored May 8, 2020
1 parent 69e732a commit df14b5b
Show file tree
Hide file tree
Showing 12 changed files with 208 additions and 51 deletions.
4 changes: 0 additions & 4 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -495,10 +495,6 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
throw new Error('Not yet implemented');
}

export function makeServerId(): OpaqueIDType {
throw new Error('Not yet implemented');
}

export function beforeActiveInstanceBlur() {
// noop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,162 @@ describe('ReactDOMServerHooks', () => {
);
});

it('useOpaqueIdentifier identifierPrefix works for server renderer and does not clash', async () => {
function ChildTwo({id}) {
return <div id={id}>Child Three</div>;
}
function App() {
const id = useOpaqueIdentifier();
const idTwo = useOpaqueIdentifier();

return (
<div>
<div aria-labelledby={id}>Chid One</div>
<ChildTwo id={id} />
<div aria-labelledby={idTwo}>Child Three</div>
<div id={idTwo}>Child Four</div>
</div>
);
}

const containerOne = document.createElement('div');
document.body.append(containerOne);

containerOne.innerHTML = ReactDOMServer.renderToString(<App />, {
identifierPrefix: 'one',
});

const containerTwo = document.createElement('div');
document.body.append(containerTwo);

containerTwo.innerHTML = ReactDOMServer.renderToString(<App />, {
identifierPrefix: 'two',
});

expect(document.body.children.length).toEqual(2);
const childOne = document.body.children[0];
const childTwo = document.body.children[1];

expect(
childOne.children[0].children[0].getAttribute('aria-labelledby'),
).toEqual(childOne.children[0].children[1].getAttribute('id'));
expect(
childOne.children[0].children[2].getAttribute('aria-labelledby'),
).toEqual(childOne.children[0].children[3].getAttribute('id'));

expect(
childOne.children[0].children[0].getAttribute('aria-labelledby'),
).not.toEqual(
childOne.children[0].children[2].getAttribute('aria-labelledby'),
);

expect(
childOne.children[0].children[0]
.getAttribute('aria-labelledby')
.startsWith('one'),
).toBe(true);
expect(
childOne.children[0].children[2]
.getAttribute('aria-labelledby')
.includes('one'),
).toBe(true);

expect(
childTwo.children[0].children[0].getAttribute('aria-labelledby'),
).toEqual(childTwo.children[0].children[1].getAttribute('id'));
expect(
childTwo.children[0].children[2].getAttribute('aria-labelledby'),
).toEqual(childTwo.children[0].children[3].getAttribute('id'));

expect(
childTwo.children[0].children[0].getAttribute('aria-labelledby'),
).not.toEqual(
childTwo.children[0].children[2].getAttribute('aria-labelledby'),
);

expect(
childTwo.children[0].children[0]
.getAttribute('aria-labelledby')
.startsWith('two'),
).toBe(true);
expect(
childTwo.children[0].children[2]
.getAttribute('aria-labelledby')
.startsWith('two'),
).toBe(true);
});

it('useOpaqueIdentifier identifierPrefix works for multiple reads on a streaming server renderer', async () => {
function ChildTwo() {
const id = useOpaqueIdentifier();

return <div id={id}>Child Two</div>;
}

function App() {
const id = useOpaqueIdentifier();

return (
<>
<div id={id}>Child One</div>
<ChildTwo />
<div aria-labelledby={id}>Aria One</div>
</>
);
}

const container = document.createElement('div');
document.body.append(container);

const streamOne = ReactDOMServer.renderToNodeStream(<App />, {
identifierPrefix: 'one',
}).setEncoding('utf8');
const streamTwo = ReactDOMServer.renderToNodeStream(<App />, {
identifierPrefix: 'two',
}).setEncoding('utf8');

const containerOne = document.createElement('div');
const containerTwo = document.createElement('div');

streamOne._read(10);
streamTwo._read(10);

containerOne.innerHTML = streamOne.read();
containerTwo.innerHTML = streamTwo.read();

expect(containerOne.children[0].getAttribute('id')).not.toEqual(
containerOne.children[1].getAttribute('id'),
);
expect(containerTwo.children[0].getAttribute('id')).not.toEqual(
containerTwo.children[1].getAttribute('id'),
);
expect(containerOne.children[0].getAttribute('id')).not.toEqual(
containerTwo.children[0].getAttribute('id'),
);
expect(
containerOne.children[0].getAttribute('id').includes('one'),
).toBe(true);
expect(
containerOne.children[1].getAttribute('id').includes('one'),
).toBe(true);
expect(
containerTwo.children[0].getAttribute('id').includes('two'),
).toBe(true);
expect(
containerTwo.children[1].getAttribute('id').includes('two'),
).toBe(true);

expect(containerOne.children[1].getAttribute('id')).not.toEqual(
containerTwo.children[1].getAttribute('id'),
);
expect(containerOne.children[0].getAttribute('id')).toEqual(
containerOne.children[2].getAttribute('aria-labelledby'),
);
expect(containerTwo.children[0].getAttribute('id')).toEqual(
containerTwo.children[2].getAttribute('aria-labelledby'),
);
});

it('useOpaqueIdentifier: IDs match when, after hydration, a new component that uses the ID is rendered', async () => {
let _setShowDiv;
function App() {
Expand Down
5 changes: 0 additions & 5 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -1102,11 +1102,6 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
};
}

let serverId: number = 0;
export function makeServerId(): OpaqueIDType {
return 'R:' + (serverId++).toString(36);
}

export function isOpaqueHydratingObject(value: mixed): boolean {
return (
value !== null &&
Expand Down
17 changes: 11 additions & 6 deletions packages/react-dom/src/server/ReactDOMNodeStreamRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,23 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import type {ServerOptions} from './ReactPartialRenderer';

import {Readable} from 'stream';

import ReactPartialRenderer from './ReactPartialRenderer';

// This is a Readable Node.js stream which wraps the ReactDOMPartialRenderer.
class ReactMarkupReadableStream extends Readable {
constructor(element, makeStaticMarkup) {
constructor(element, makeStaticMarkup, options) {
// Calls the stream.Readable(options) constructor. Consider exposing built-in
// features like highWaterMark in the future.
super({});
this.partialRenderer = new ReactPartialRenderer(element, makeStaticMarkup);
this.partialRenderer = new ReactPartialRenderer(
element,
makeStaticMarkup,
options,
);
}

_destroy(err, callback) {
Expand All @@ -36,15 +41,15 @@ class ReactMarkupReadableStream extends Readable {
* server.
* See https://reactjs.org/docs/react-dom-server.html#rendertonodestream
*/
export function renderToNodeStream(element) {
return new ReactMarkupReadableStream(element, false);
export function renderToNodeStream(element, options?: ServerOptions) {
return new ReactMarkupReadableStream(element, false, options);
}

/**
* Similar to renderToNodeStream, except this doesn't create extra DOM attributes
* such as data-react-id that React uses internally.
* See https://reactjs.org/docs/react-dom-server.html#rendertostaticnodestream
*/
export function renderToStaticNodeStream(element) {
return new ReactMarkupReadableStream(element, true);
export function renderToStaticNodeStream(element, options?: ServerOptions) {
return new ReactMarkupReadableStream(element, true, options);
}
9 changes: 5 additions & 4 deletions packages/react-dom/src/server/ReactDOMStringRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
* LICENSE file in the root directory of this source tree.
*/

import type {ServerOptions} from './ReactPartialRenderer';
import ReactPartialRenderer from './ReactPartialRenderer';

/**
* Render a ReactElement to its initial HTML. This should only be used on the
* server.
* See https://reactjs.org/docs/react-dom-server.html#rendertostring
*/
export function renderToString(element) {
const renderer = new ReactPartialRenderer(element, false);
export function renderToString(element, options?: ServerOptions) {
const renderer = new ReactPartialRenderer(element, false, options);
try {
const markup = renderer.read(Infinity);
return markup;
Expand All @@ -27,8 +28,8 @@ export function renderToString(element) {
* such as data-react-id that React uses internally.
* See https://reactjs.org/docs/react-dom-server.html#rendertostaticmarkup
*/
export function renderToStaticMarkup(element) {
const renderer = new ReactPartialRenderer(element, true);
export function renderToStaticMarkup(element, options?: ServerOptions) {
const renderer = new ReactPartialRenderer(element, true, options);
try {
const markup = renderer.read(Infinity);
return markup;
Expand Down
28 changes: 22 additions & 6 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ import {
prepareToUseHooks,
finishHooks,
Dispatcher,
currentThreadID,
setCurrentThreadID,
currentPartialRenderer,
setCurrentPartialRenderer,
} from './ReactPartialRendererHooks';
import {
Namespaces,
Expand All @@ -79,6 +79,10 @@ import {validateProperties as validateARIAProperties} from '../shared/ReactDOMIn
import {validateProperties as validateInputProperties} from '../shared/ReactDOMNullInputValuePropHook';
import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook';

export type ServerOptions = {
identifierPrefix?: string,
};

// Based on reading the React.Children implementation. TODO: type this somewhere?
type ReactNode = string | number | ReactElement;
type FlatReactChildren = Array<null | ReactNode>;
Expand Down Expand Up @@ -726,7 +730,14 @@ class ReactDOMServerRenderer {
contextValueStack: Array<any>;
contextProviderStack: ?Array<ReactProvider<any>>; // DEV-only

constructor(children: mixed, makeStaticMarkup: boolean) {
uniqueID: number;
identifierPrefix: string;

constructor(
children: mixed,
makeStaticMarkup: boolean,
options?: ServerOptions,
) {
const flatChildren = flattenTopLevelChildren(children);

const topFrame: Frame = {
Expand Down Expand Up @@ -754,6 +765,11 @@ class ReactDOMServerRenderer {
this.contextIndex = -1;
this.contextStack = [];
this.contextValueStack = [];

// useOpaqueIdentifier ID
this.uniqueID = 0;
this.identifierPrefix = (options && options.identifierPrefix) || '';

if (__DEV__) {
this.contextProviderStack = [];
}
Expand Down Expand Up @@ -837,8 +853,8 @@ class ReactDOMServerRenderer {
return null;
}

const prevThreadID = currentThreadID;
setCurrentThreadID(this.threadID);
const prevPartialRenderer = currentPartialRenderer;
setCurrentPartialRenderer(this);
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = Dispatcher;
try {
Expand Down Expand Up @@ -935,7 +951,7 @@ class ReactDOMServerRenderer {
return out[0];
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
setCurrentThreadID(prevThreadID);
setCurrentPartialRenderer(prevPartialRenderer);
}
}

Expand Down
Loading

0 comments on commit df14b5b

Please sign in to comment.