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

Store unique ID counter on SSR request object and Add Prefix for separate renders #18576

Merged
merged 1 commit into from
May 8, 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
4 changes: 0 additions & 4 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -489,10 +489,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 @@ -1004,6 +1004,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 @@ -1078,11 +1078,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