Add support for re-entrant SSR stacks (#13181)
* Add failing tests * Fix re-entrancy in ReactDOMServer
Dan Abramov committed
Jul 11, 2018 at 19:43 UTC
6ebc8f3c07d8aadea127532fd33637cb091a9e66
3 files changed
+205
-42
packages/react-dom/src/__tests__/ReactDOM-test.js
+72
-3
@@ -9,11 +9,20 @@
9
10
'use strict';
11
12
-let React = require('react');
13
-let ReactDOM = require('react-dom');
14
-const ReactTestUtils = require('react-dom/test-utils');
12
+let React;
13
+let ReactDOM;
14
+let ReactDOMServer;
15
+let ReactTestUtils;
16
17
describe('ReactDOM', () => {
18
+ beforeEach(() => {
19
+ jest.resetModules();
20
+ React = require('react');
21
+ ReactDOM = require('react-dom');
22
+ ReactDOMServer = require('react-dom/server');
23
+ ReactTestUtils = require('react-dom/test-utils');
24
+ });
25
+
26
// TODO: uncomment this test once we can run in phantom, which
27
// supports real submit events.
28
/*
@@ -446,4 +455,64 @@ describe('ReactDOM', () => {
455
global.requestAnimationFrame = previousRAF;
456
}
457
});
458
+
459
+ it('reports stacks with re-entrant renderToString() calls on the client', () => {
460
+ function Child2(props) {
461
+ return <span ariaTypo3="no">{props.children}</span>;
462
+ }
463
+
464
+ function App2() {
465
+ return (
466
+ <Child2>
467
+ {ReactDOMServer.renderToString(<blink ariaTypo2="no" />)}
468
+ </Child2>
469
+ );
470
+ }
471
+
472
+ function Child() {
473
+ return (
474
+ <span ariaTypo4="no">{ReactDOMServer.renderToString(<App2 />)}</span>
475
+ );
476
+ }
477
+
478
+ function ServerEntry() {
479
+ return ReactDOMServer.renderToString(<Child />);
480
+ }
481
+
482
+ function App() {
483
+ return (
484
+ <div>
485
+ <span ariaTypo="no" />
486
+ <ServerEntry />
487
+ <font ariaTypo5="no" />
488
+ </div>
489
+ );
490
+ }
491
+
492
+ const container = document.createElement('div');
493
+ expect(() => ReactDOM.render(<App />, container)).toWarnDev([
494
+ // ReactDOM(App > div > span)
495
+ 'Invalid ARIA attribute `ariaTypo`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
496
+ ' in span (at **)\n' +
497
+ ' in div (at **)\n' +
498
+ ' in App (at **)',
499
+ // ReactDOM(App > div > ServerEntry) >>> ReactDOMServer(Child) >>> ReactDOMServer(App2) >>> ReactDOMServer(blink)
500
+ 'Invalid ARIA attribute `ariaTypo2`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
501
+ ' in blink (at **)',
502
+ // ReactDOM(App > div > ServerEntry) >>> ReactDOMServer(Child) >>> ReactDOMServer(App2 > Child2 > span)
503
+ 'Invalid ARIA attribute `ariaTypo3`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
504
+ ' in span (at **)\n' +
505
+ ' in Child2 (at **)\n' +
506
+ ' in App2 (at **)',
507
+ // ReactDOM(App > div > ServerEntry) >>> ReactDOMServer(Child > span)
508
+ 'Invalid ARIA attribute `ariaTypo4`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
509
+ ' in span (at **)\n' +
510
+ ' in Child (at **)',
511
+ // ReactDOM(App > div > font)
512
+ 'Invalid ARIA attribute `ariaTypo5`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
513
+ ' in font (at **)\n' +
514
+ ' in div (at **)\n' +
515
+ ' in App (at **)',
516
+ ]);
517
+ });
518
});
packages/react-dom/src/__tests__/ReactServerRendering-test.js
+57
@@ -720,4 +720,61 @@ describe('ReactDOMServer', () => {
720
' in App (at **)',
721
]);
722
});
723
+
724
+ it('reports stacks with re-entrant renderToString() calls', () => {
725
+ function Child2(props) {
726
+ return <span ariaTypo3="no">{props.children}</span>;
727
+ }
728
+
729
+ function App2() {
730
+ return (
731
+ <Child2>
732
+ {ReactDOMServer.renderToString(<blink ariaTypo2="no" />)}
733
+ </Child2>
734
+ );
735
+ }
736
+
737
+ function Child() {
738
+ return (
739
+ <span ariaTypo4="no">{ReactDOMServer.renderToString(<App2 />)}</span>
740
+ );
741
+ }
742
+
743
+ function App() {
744
+ return (
745
+ <div>
746
+ <span ariaTypo="no" />
747
+ <Child />
748
+ <font ariaTypo5="no" />
749
+ </div>
750
+ );
751
+ }
752
+
753
+ expect(() => ReactDOMServer.renderToString(<App />)).toWarnDev([
754
+ // ReactDOMServer(App > div > span)
755
+ 'Invalid ARIA attribute `ariaTypo`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
756
+ ' in span (at **)\n' +
757
+ ' in div (at **)\n' +
758
+ ' in App (at **)',
759
+ // ReactDOMServer(App > div > Child) >>> ReactDOMServer(App2) >>> ReactDOMServer(blink)
760
+ 'Invalid ARIA attribute `ariaTypo2`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
761
+ ' in blink (at **)',
762
+ // ReactDOMServer(App > div > Child) >>> ReactDOMServer(App2 > Child2 > span)
763
+ 'Invalid ARIA attribute `ariaTypo3`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
764
+ ' in span (at **)\n' +
765
+ ' in Child2 (at **)\n' +
766
+ ' in App2 (at **)',
767
+ // ReactDOMServer(App > div > Child > span)
768
+ 'Invalid ARIA attribute `ariaTypo4`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
769
+ ' in span (at **)\n' +
770
+ ' in Child (at **)\n' +
771
+ ' in div (at **)\n' +
772
+ ' in App (at **)',
773
+ // ReactDOMServer(App > div > font)
774
+ 'Invalid ARIA attribute `ariaTypo5`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
775
+ ' in font (at **)\n' +
776
+ ' in div (at **)\n' +
777
+ ' in App (at **)',
778
+ ]);
779
+ });
780
});
packages/react-dom/src/server/ReactPartialRenderer.js
+76
-39
@@ -61,16 +61,20 @@ type FlatReactChildren = Array<null | ReactNode>;
61
type toArrayType = (children: mixed) => FlatReactChildren;
62
const toArray = ((React.Children.toArray: any): toArrayType);
63
64
-let currentDebugStack;
65
-let currentDebugElementStack;
66
-
67
-let getStackAddendum = () => '';
64
+// This is only used in DEV.
65
+// Each entry is `this.stack` from a currently executing renderer instance.
66
+// (There may be more than one because ReactDOMServer is reentrant).
67
+// Each stack is an array of frames which may contain nested stacks of elements.
68
+let currentDebugStacks = [];
69
+
70
+let prevGetCurrentStackImpl = null;
71
+let getCurrentServerStackImpl = () => '';
72
let describeStackFrame = element => '';
73
74
let validatePropertiesInDevelopment = (type, props) => {};
71
-let setCurrentDebugStack = (stack: Array<Frame>) => {};
75
+let pushCurrentDebugStack = (stack: Array<Frame>) => {};
76
let pushElementToDebugStack = (element: ReactElement) => {};
73
-let resetCurrentDebugStack = () => {};
77
+let popCurrentDebugStack = () => {};
78
79
if (__DEV__) {
80
validatePropertiesInDevelopment = function(type, props) {
@@ -87,34 +91,55 @@ if (__DEV__) {
91
return describeComponentFrame(name, source, ownerName);
92
};
93
90
- currentDebugStack = null;
91
- currentDebugElementStack = null;
92
- setCurrentDebugStack = function(stack: Array<Frame>) {
93
- const frame: Frame = stack[stack.length - 1];
94
- currentDebugElementStack = ((frame: any): FrameDev).debugElementStack;
95
- // We are about to enter a new composite stack, reset the array.
96
- currentDebugElementStack.length = 0;
97
- currentDebugStack = stack;
98
- ReactDebugCurrentFrame.getCurrentStack = getStackAddendum;
94
+ pushCurrentDebugStack = function(stack: Array<Frame>) {
95
+ currentDebugStacks.push(stack);
96
+
97
+ if (currentDebugStacks.length === 1) {
98
+ // We are entering a server renderer.
99
+ // Remember the previous (e.g. client) global stack implementation.
100
+ prevGetCurrentStackImpl = ReactDebugCurrentFrame.getCurrentStack;
101
+ ReactDebugCurrentFrame.getCurrentStack = getCurrentServerStackImpl;
102
+ }
103
};
104
+
105
pushElementToDebugStack = function(element: ReactElement) {
101
- if (currentDebugElementStack !== null) {
102
- currentDebugElementStack.push(element);
103
- }
106
+ // For the innermost executing ReactDOMServer call,
107
+ const stack = currentDebugStacks[currentDebugStacks.length - 1];
108
+ // Take the innermost executing frame (e.g. <Foo>),
109
+ const frame: Frame = stack[stack.length - 1];
110
+ // and record that it has one more element associated with it.
111
+ ((frame: any): FrameDev).debugElementStack.push(element);
112
+ // We only need this because we tail-optimize single-element
113
+ // children and directly handle them in an inner loop instead of
114
+ // creating separate frames for them.
115
};
105
- resetCurrentDebugStack = function() {
106
- currentDebugElementStack = null;
107
- currentDebugStack = null;
108
- ReactDebugCurrentFrame.getCurrentStack = null;
116
+
117
+ popCurrentDebugStack = function() {
118
+ currentDebugStacks.pop();
119
+
120
+ if (currentDebugStacks.length === 0) {
121
+ // We are exiting the server renderer.
122
+ // Restore the previous (e.g. client) global stack implementation.
123
+ ReactDebugCurrentFrame.getCurrentStack = prevGetCurrentStackImpl;
124
+ prevGetCurrentStackImpl = null;
125
+ }
126
};
110
- getStackAddendum = function(): null | string {
111
- if (currentDebugStack === null) {
127
+
128
+ getCurrentServerStackImpl = function(): string {
129
+ if (currentDebugStacks.length === 0) {
130
+ // Nothing is currently rendering.
131
return '';
132
}
133
+ // ReactDOMServer is reentrant so there may be multiple calls at the same time.
134
+ // Take the frames from the innermost call which is the last in the array.
135
+ let frames = currentDebugStacks[currentDebugStacks.length - 1];
136
let stack = '';
115
- let debugStack = currentDebugStack;
116
- for (let i = debugStack.length - 1; i >= 0; i--) {
117
- const frame: Frame = debugStack[i];
137
+ // Go through every frame in the stack from the innermost one.
138
+ for (let i = frames.length - 1; i >= 0; i--) {
139
+ const frame: Frame = frames[i];
140
+ // Every frame might have more than one debug element stack entry associated with it.
141
+ // This is because single-child nesting doesn't create materialized frames.
142
+ // Instead it would push them through `pushElementToDebugStack()`.
143
let debugElementStack = ((frame: any): FrameDev).debugElementStack;
144
for (let ii = debugElementStack.length - 1; ii >= 0; ii--) {
145
stack += describeStackFrame(debugElementStack[ii]);
@@ -180,7 +205,7 @@ function createMarkupForStyles(styles): string | null {
205
const styleValue = styles[styleName];
206
if (__DEV__) {
207
if (!isCustomProperty) {
183
- warnValidStyle(styleName, styleValue, getStackAddendum);
208
+ warnValidStyle(styleName, styleValue, getCurrentServerStackImpl);
209
}
210
}
211
if (styleValue != null) {
@@ -305,7 +330,13 @@ function maskContext(type, context) {
330
331
function checkContextTypes(typeSpecs, values, location: string) {
332
if (__DEV__) {
308
- checkPropTypes(typeSpecs, values, location, 'Component', getStackAddendum);
333
+ checkPropTypes(
334
+ typeSpecs,
335
+ values,
336
+ location,
337
+ 'Component',
338
+ getCurrentServerStackImpl,
339
+ );
340
}
341
}
342
@@ -774,12 +805,18 @@ class ReactDOMServerRenderer {
805
}
806
const child = frame.children[frame.childIndex++];
807
if (__DEV__) {
777
- setCurrentDebugStack(this.stack);
778
- }
779
- out += this.render(child, frame.context, frame.domNamespace);
780
- if (__DEV__) {
781
- // TODO: Handle reentrant server render calls. This doesn't.
782
- resetCurrentDebugStack();
808
+ pushCurrentDebugStack(this.stack);
809
+ // We're starting work on this frame, so reset its inner stack.
810
+ ((frame: any): FrameDev).debugElementStack.length = 0;
811
+ try {
812
+ // Be careful! Make sure this matches the PROD path below.
813
+ out += this.render(child, frame.context, frame.domNamespace);
814
+ } finally {
815
+ popCurrentDebugStack();
816
+ }
817
+ } else {
818
+ // Be careful! Make sure this matches the DEV path above.
819
+ out += this.render(child, frame.context, frame.domNamespace);
820
}
821
}
822
return out;
@@ -1005,7 +1042,7 @@ class ReactDOMServerRenderer {
1042
ReactControlledValuePropTypes.checkPropTypes(
1043
'input',
1044
props,
1008
- getStackAddendum,
1045
+ getCurrentServerStackImpl,
1046
);
1047
1048
if (
@@ -1063,7 +1100,7 @@ class ReactDOMServerRenderer {
1100
ReactControlledValuePropTypes.checkPropTypes(
1101
'textarea',
1102
props,
1066
- getStackAddendum,
1103
+ getCurrentServerStackImpl,
1104
);
1105
if (
1106
props.value !== undefined &&
@@ -1124,7 +1161,7 @@ class ReactDOMServerRenderer {
1161
ReactControlledValuePropTypes.checkPropTypes(
1162
'select',
1163
props,
1127
- getStackAddendum,
1164
+ getCurrentServerStackImpl,
1165
);
1166
1167
for (let i = 0; i < valuePropNames.length; i++) {
@@ -1215,7 +1252,7 @@ class ReactDOMServerRenderer {
1252
validatePropertiesInDevelopment(tag, props);
1253
}
1254
1218
- assertValidProps(tag, props, getStackAddendum);
1255
+ assertValidProps(tag, props, getCurrentServerStackImpl);
1256
1257
let out = createOpenTagMarkup(
1258
element.type,