@samitouri / QOS-React-2 / commits / df14b5bcc1

add new IDs for each each server renderer instance and prefixes to distinguish 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.

Luna Ruan committed May 7, 2020 at 20:46 UTC df14b5bcc163516fc0f1ad35e9b93732c66c1085
12 files changed +208 -51
packages/react-art/src/ReactARTHostConfig.js
-4
@@ -495,10 +495,6 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
495 throw new Error('Not yet implemented');
496 }
497
498 -export function makeServerId(): OpaqueIDType {
499 - throw new Error('Not yet implemented');
500 -}
501 -
498 export function beforeActiveInstanceBlur() {
499 // noop
500 }
packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js
+156
@@ -1032,6 +1032,162 @@ describe('ReactDOMServerHooks', () => {
1032 );
1033 });
1034
1035 + it('useOpaqueIdentifier identifierPrefix works for server renderer and does not clash', async () => {
1036 + function ChildTwo({id}) {
1037 + return <div id={id}>Child Three</div>;
1038 + }
1039 + function App() {
1040 + const id = useOpaqueIdentifier();
1041 + const idTwo = useOpaqueIdentifier();
1042 +
1043 + return (
1044 + <div>
1045 + <div aria-labelledby={id}>Chid One</div>
1046 + <ChildTwo id={id} />
1047 + <div aria-labelledby={idTwo}>Child Three</div>
1048 + <div id={idTwo}>Child Four</div>
1049 + </div>
1050 + );
1051 + }
1052 +
1053 + const containerOne = document.createElement('div');
1054 + document.body.append(containerOne);
1055 +
1056 + containerOne.innerHTML = ReactDOMServer.renderToString(<App />, {
1057 + identifierPrefix: 'one',
1058 + });
1059 +
1060 + const containerTwo = document.createElement('div');
1061 + document.body.append(containerTwo);
1062 +
1063 + containerTwo.innerHTML = ReactDOMServer.renderToString(<App />, {
1064 + identifierPrefix: 'two',
1065 + });
1066 +
1067 + expect(document.body.children.length).toEqual(2);
1068 + const childOne = document.body.children[0];
1069 + const childTwo = document.body.children[1];
1070 +
1071 + expect(
1072 + childOne.children[0].children[0].getAttribute('aria-labelledby'),
1073 + ).toEqual(childOne.children[0].children[1].getAttribute('id'));
1074 + expect(
1075 + childOne.children[0].children[2].getAttribute('aria-labelledby'),
1076 + ).toEqual(childOne.children[0].children[3].getAttribute('id'));
1077 +
1078 + expect(
1079 + childOne.children[0].children[0].getAttribute('aria-labelledby'),
1080 + ).not.toEqual(
1081 + childOne.children[0].children[2].getAttribute('aria-labelledby'),
1082 + );
1083 +
1084 + expect(
1085 + childOne.children[0].children[0]
1086 + .getAttribute('aria-labelledby')
1087 + .startsWith('one'),
1088 + ).toBe(true);
1089 + expect(
1090 + childOne.children[0].children[2]
1091 + .getAttribute('aria-labelledby')
1092 + .includes('one'),
1093 + ).toBe(true);
1094 +
1095 + expect(
1096 + childTwo.children[0].children[0].getAttribute('aria-labelledby'),
1097 + ).toEqual(childTwo.children[0].children[1].getAttribute('id'));
1098 + expect(
1099 + childTwo.children[0].children[2].getAttribute('aria-labelledby'),
1100 + ).toEqual(childTwo.children[0].children[3].getAttribute('id'));
1101 +
1102 + expect(
1103 + childTwo.children[0].children[0].getAttribute('aria-labelledby'),
1104 + ).not.toEqual(
1105 + childTwo.children[0].children[2].getAttribute('aria-labelledby'),
1106 + );
1107 +
1108 + expect(
1109 + childTwo.children[0].children[0]
1110 + .getAttribute('aria-labelledby')
1111 + .startsWith('two'),
1112 + ).toBe(true);
1113 + expect(
1114 + childTwo.children[0].children[2]
1115 + .getAttribute('aria-labelledby')
1116 + .startsWith('two'),
1117 + ).toBe(true);
1118 + });
1119 +
1120 + it('useOpaqueIdentifier identifierPrefix works for multiple reads on a streaming server renderer', async () => {
1121 + function ChildTwo() {
1122 + const id = useOpaqueIdentifier();
1123 +
1124 + return <div id={id}>Child Two</div>;
1125 + }
1126 +
1127 + function App() {
1128 + const id = useOpaqueIdentifier();
1129 +
1130 + return (
1131 + <>
1132 + <div id={id}>Child One</div>
1133 + <ChildTwo />
1134 + <div aria-labelledby={id}>Aria One</div>
1135 + </>
1136 + );
1137 + }
1138 +
1139 + const container = document.createElement('div');
1140 + document.body.append(container);
1141 +
1142 + const streamOne = ReactDOMServer.renderToNodeStream(<App />, {
1143 + identifierPrefix: 'one',
1144 + }).setEncoding('utf8');
1145 + const streamTwo = ReactDOMServer.renderToNodeStream(<App />, {
1146 + identifierPrefix: 'two',
1147 + }).setEncoding('utf8');
1148 +
1149 + const containerOne = document.createElement('div');
1150 + const containerTwo = document.createElement('div');
1151 +
1152 + streamOne._read(10);
1153 + streamTwo._read(10);
1154 +
1155 + containerOne.innerHTML = streamOne.read();
1156 + containerTwo.innerHTML = streamTwo.read();
1157 +
1158 + expect(containerOne.children[0].getAttribute('id')).not.toEqual(
1159 + containerOne.children[1].getAttribute('id'),
1160 + );
1161 + expect(containerTwo.children[0].getAttribute('id')).not.toEqual(
1162 + containerTwo.children[1].getAttribute('id'),
1163 + );
1164 + expect(containerOne.children[0].getAttribute('id')).not.toEqual(
1165 + containerTwo.children[0].getAttribute('id'),
1166 + );
1167 + expect(
1168 + containerOne.children[0].getAttribute('id').includes('one'),
1169 + ).toBe(true);
1170 + expect(
1171 + containerOne.children[1].getAttribute('id').includes('one'),
1172 + ).toBe(true);
1173 + expect(
1174 + containerTwo.children[0].getAttribute('id').includes('two'),
1175 + ).toBe(true);
1176 + expect(
1177 + containerTwo.children[1].getAttribute('id').includes('two'),
1178 + ).toBe(true);
1179 +
1180 + expect(containerOne.children[1].getAttribute('id')).not.toEqual(
1181 + containerTwo.children[1].getAttribute('id'),
1182 + );
1183 + expect(containerOne.children[0].getAttribute('id')).toEqual(
1184 + containerOne.children[2].getAttribute('aria-labelledby'),
1185 + );
1186 + expect(containerTwo.children[0].getAttribute('id')).toEqual(
1187 + containerTwo.children[2].getAttribute('aria-labelledby'),
1188 + );
1189 + });
1190 +
1191 it('useOpaqueIdentifier: IDs match when, after hydration, a new component that uses the ID is rendered', async () => {
1192 let _setShowDiv;
1193 function App() {
packages/react-dom/src/client/ReactDOMHostConfig.js
-5
@@ -1102,11 +1102,6 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
1102 };
1103 }
1104
1105 -let serverId: number = 0;
1106 -export function makeServerId(): OpaqueIDType {
1107 - return 'R:' + (serverId++).toString(36);
1108 -}
1109 -
1105 export function isOpaqueHydratingObject(value: mixed): boolean {
1106 return (
1107 value !== null &&
packages/react-dom/src/server/ReactDOMNodeStreamRenderer.js
+11 -6
@@ -4,6 +4,7 @@
4 * This source code is licensed under the MIT license found in the
5 * LICENSE file in the root directory of this source tree.
6 */
7 +import type {ServerOptions} from './ReactPartialRenderer';
8
9 import {Readable} from 'stream';
10
@@ -11,11 +12,15 @@ import ReactPartialRenderer from './ReactPartialRenderer';
12
13 // This is a Readable Node.js stream which wraps the ReactDOMPartialRenderer.
14 class ReactMarkupReadableStream extends Readable {
14 - constructor(element, makeStaticMarkup) {
15 + constructor(element, makeStaticMarkup, options) {
16 // Calls the stream.Readable(options) constructor. Consider exposing built-in
17 // features like highWaterMark in the future.
18 super({});
18 - this.partialRenderer = new ReactPartialRenderer(element, makeStaticMarkup);
19 + this.partialRenderer = new ReactPartialRenderer(
20 + element,
21 + makeStaticMarkup,
22 + options,
23 + );
24 }
25
26 _destroy(err, callback) {
@@ -36,8 +41,8 @@ class ReactMarkupReadableStream extends Readable {
41 * server.
42 * See https://reactjs.org/docs/react-dom-server.html#rendertonodestream
43 */
39 -export function renderToNodeStream(element) {
40 - return new ReactMarkupReadableStream(element, false);
44 +export function renderToNodeStream(element, options?: ServerOptions) {
45 + return new ReactMarkupReadableStream(element, false, options);
46 }
47
48 /**
@@ -45,6 +50,6 @@ export function renderToNodeStream(element) {
50 * such as data-react-id that React uses internally.
51 * See https://reactjs.org/docs/react-dom-server.html#rendertostaticnodestream
52 */
48 -export function renderToStaticNodeStream(element) {
49 - return new ReactMarkupReadableStream(element, true);
53 +export function renderToStaticNodeStream(element, options?: ServerOptions) {
54 + return new ReactMarkupReadableStream(element, true, options);
55 }
packages/react-dom/src/server/ReactDOMStringRenderer.js
+5 -4
@@ -5,6 +5,7 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 +import type {ServerOptions} from './ReactPartialRenderer';
9 import ReactPartialRenderer from './ReactPartialRenderer';
10
11 /**
@@ -12,8 +13,8 @@ import ReactPartialRenderer from './ReactPartialRenderer';
13 * server.
14 * See https://reactjs.org/docs/react-dom-server.html#rendertostring
15 */
15 -export function renderToString(element) {
16 - const renderer = new ReactPartialRenderer(element, false);
16 +export function renderToString(element, options?: ServerOptions) {
17 + const renderer = new ReactPartialRenderer(element, false, options);
18 try {
19 const markup = renderer.read(Infinity);
20 return markup;
@@ -27,8 +28,8 @@ export function renderToString(element) {
28 * such as data-react-id that React uses internally.
29 * See https://reactjs.org/docs/react-dom-server.html#rendertostaticmarkup
30 */
30 -export function renderToStaticMarkup(element) {
31 - const renderer = new ReactPartialRenderer(element, true);
31 +export function renderToStaticMarkup(element, options?: ServerOptions) {
32 + const renderer = new ReactPartialRenderer(element, true, options);
33 try {
34 const markup = renderer.read(Infinity);
35 return markup;
packages/react-dom/src/server/ReactPartialRenderer.js
+22 -6
@@ -60,8 +60,8 @@ import {
60 prepareToUseHooks,
61 finishHooks,
62 Dispatcher,
63 - currentThreadID,
64 - setCurrentThreadID,
63 + currentPartialRenderer,
64 + setCurrentPartialRenderer,
65 } from './ReactPartialRendererHooks';
66 import {
67 Namespaces,
@@ -79,6 +79,10 @@ import {validateProperties as validateARIAProperties} from '../shared/ReactDOMIn
79 import {validateProperties as validateInputProperties} from '../shared/ReactDOMNullInputValuePropHook';
80 import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook';
81
82 +export type ServerOptions = {
83 + identifierPrefix?: string,
84 +};
85 +
86 // Based on reading the React.Children implementation. TODO: type this somewhere?
87 type ReactNode = string | number | ReactElement;
88 type FlatReactChildren = Array<null | ReactNode>;
@@ -726,7 +730,14 @@ class ReactDOMServerRenderer {
730 contextValueStack: Array<any>;
731 contextProviderStack: ?Array<ReactProvider<any>>; // DEV-only
732
729 - constructor(children: mixed, makeStaticMarkup: boolean) {
733 + uniqueID: number;
734 + identifierPrefix: string;
735 +
736 + constructor(
737 + children: mixed,
738 + makeStaticMarkup: boolean,
739 + options?: ServerOptions,
740 + ) {
741 const flatChildren = flattenTopLevelChildren(children);
742
743 const topFrame: Frame = {
@@ -754,6 +765,11 @@ class ReactDOMServerRenderer {
765 this.contextIndex = -1;
766 this.contextStack = [];
767 this.contextValueStack = [];
768 +
769 + // useOpaqueIdentifier ID
770 + this.uniqueID = 0;
771 + this.identifierPrefix = (options && options.identifierPrefix) || '';
772 +
773 if (__DEV__) {
774 this.contextProviderStack = [];
775 }
@@ -837,8 +853,8 @@ class ReactDOMServerRenderer {
853 return null;
854 }
855
840 - const prevThreadID = currentThreadID;
841 - setCurrentThreadID(this.threadID);
856 + const prevPartialRenderer = currentPartialRenderer;
857 + setCurrentPartialRenderer(this);
858 const prevDispatcher = ReactCurrentDispatcher.current;
859 ReactCurrentDispatcher.current = Dispatcher;
860 try {
@@ -935,7 +951,7 @@ class ReactDOMServerRenderer {
951 return out[0];
952 } finally {
953 ReactCurrentDispatcher.current = prevDispatcher;
938 - setCurrentThreadID(prevThreadID);
954 + setCurrentPartialRenderer(prevPartialRenderer);
955 }
956 }
957
packages/react-dom/src/server/ReactPartialRendererHooks.js
+13 -10
@@ -8,8 +8,6 @@
8 */
9
10 import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactInternalTypes';
11 -import type {ThreadID} from './ReactThreadIDAllocator';
12 -import type {OpaqueIDType} from 'react-reconciler/src/ReactFiberHostConfig';
11
12 import type {
13 MutableSource,
@@ -19,9 +17,9 @@ import type {
17 ReactEventResponderListener,
18 } from 'shared/ReactTypes';
19 import type {SuspenseConfig} from 'react-reconciler/src/ReactFiberSuspenseConfig';
20 +import type PartialRenderer from './ReactPartialRenderer';
21
22 import {validateContextBounds} from './ReactPartialRendererContext';
24 -import {makeServerId} from '../client/ReactDOMHostConfig';
23
24 import invariant from 'shared/invariant';
25 import is from 'shared/objectIs';
@@ -49,6 +47,8 @@ type TimeoutConfig = {|
47 timeoutMs: number,
48 |};
49
50 +type OpaqueIDType = string;
51 +
52 let currentlyRenderingComponent: Object | null = null;
53 let firstWorkInProgressHook: Hook | null = null;
54 let workInProgressHook: Hook | null = null;
@@ -226,7 +226,7 @@ function readContext<T>(
226 context: ReactContext<T>,
227 observedBits: void | number | boolean,
228 ): T {
229 - const threadID = currentThreadID;
229 + const threadID = currentPartialRenderer.threadID;
230 validateContextBounds(context, threadID);
231 if (__DEV__) {
232 if (isInHookUserCodeInDev) {
@@ -249,7 +249,7 @@ function useContext<T>(
249 currentHookNameInDev = 'useContext';
250 }
251 resolveCurrentlyRenderingComponent();
252 - const threadID = currentThreadID;
252 + const threadID = currentPartialRenderer.threadID;
253 validateContextBounds(context, threadID);
254 return context[threadID];
255 }
@@ -494,15 +494,18 @@ function useTransition(
494 }
495
496 function useOpaqueIdentifier(): OpaqueIDType {
497 - return makeServerId();
497 + return (
498 + (currentPartialRenderer.identifierPrefix || '') +
499 + 'R:' +
500 + (currentPartialRenderer.uniqueID++).toString(36)
501 + );
502 }
503
504 function noop(): void {}
505
502 -export let currentThreadID: ThreadID = 0;
503 -
504 -export function setCurrentThreadID(threadID: ThreadID) {
505 - currentThreadID = threadID;
506 +export let currentPartialRenderer: PartialRenderer = (null: any);
507 +export function setCurrentPartialRenderer(renderer: PartialRenderer) {
508 + currentPartialRenderer = renderer;
509 }
510
511 export const Dispatcher: DispatcherType = {
packages/react-native-renderer/src/ReactFabricHostConfig.js
-4
@@ -505,10 +505,6 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
505 throw new Error('Not yet implemented');
506 }
507
508 -export function makeServerId(): OpaqueIDType {
509 - throw new Error('Not yet implemented');
510 -}
511 -
508 export function beforeActiveInstanceBlur() {
509 // noop
510 }
packages/react-native-renderer/src/ReactNativeHostConfig.js
-4
@@ -558,10 +558,6 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
558 throw new Error('Not yet implemented');
559 }
560
561 -export function makeServerId(): OpaqueIDType {
562 - throw new Error('Not yet implemented');
563 -}
564 -
561 export function beforeActiveInstanceBlur() {
562 // noop
563 }
packages/react-reconciler/src/ReactInternalTypes.js
+1 -2
@@ -29,7 +29,6 @@ import type {RootTag} from './ReactRootTags';
29 import type {TimeoutHandle, NoTimeout} from './ReactFiberHostConfig';
30 import type {Wakeable} from 'shared/ReactTypes';
31 import type {Interaction} from 'scheduler/src/Tracing';
32 -import type {OpaqueIDType} from 'react-reconciler/src/ReactFiberHostConfig';
32 import type {SuspenseConfig, TimeoutConfig} from './ReactFiberSuspenseConfig';
33
34 export type ReactPriorityLevel = 99 | 98 | 97 | 96 | 95 | 90;
@@ -356,5 +355,5 @@ export type Dispatcher = {|
355 getSnapshot: MutableSourceGetSnapshotFn<Source, Snapshot>,
356 subscribe: MutableSourceSubscribeFn<Source, Snapshot>,
357 ): Snapshot,
359 - useOpaqueIdentifier(): OpaqueIDType | void,
358 + useOpaqueIdentifier(): any,
359 |};
packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js
-1
@@ -80,7 +80,6 @@ export const makeOpaqueHydratingObject =
80 $$$hostConfig.makeOpaqueHydratingObject;
81 export const makeClientId = $$$hostConfig.makeClientId;
82 export const makeClientIdInDEV = $$$hostConfig.makeClientIdInDEV;
83 -export const makeServerId = $$$hostConfig.makeServerId;
83 export const beforeActiveInstanceBlur = $$$hostConfig.beforeActiveInstanceBlur;
84 export const afterActiveInstanceBlur = $$$hostConfig.afterActiveInstanceBlur;
85 export const preparePortalMount = $$$hostConfig.preparePortalMount;
packages/react-test-renderer/src/ReactTestHostConfig.js
-5
@@ -410,11 +410,6 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
410 };
411 }
412
413 -let serverId: number = 0;
414 -export function makeServerId(): OpaqueIDType {
415 - return 's_' + (serverId++).toString(36);
416 -}
417 -
413 export function isOpaqueHydratingObject(value: mixed): boolean {
414 return (
415 value !== null &&