Throw on unhandled SSR suspending (#16460)
* Throw on unhandled SSR suspending * Add a nicer message when the flag is off * Tweak internal refinement error message
Dan Abramov committed
Aug 19, 2019 at 19:53 UTC
56f93a7f38f25284261ee5d287aa32dcb1caf327
4 files changed
+170
-8
packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js
+87
-3
@@ -37,9 +37,11 @@ function initModules() {
37
};
38
}
39
40
-const {resetModules, serverRender} = ReactDOMServerIntegrationUtils(
41
- initModules,
42
-);
40
+const {
41
+ itThrowsWhenRendering,
42
+ resetModules,
43
+ serverRender,
44
+} = ReactDOMServerIntegrationUtils(initModules);
45
46
describe('ReactDOMServerSuspense', () => {
47
beforeEach(() => {
@@ -133,4 +135,86 @@ describe('ReactDOMServerSuspense', () => {
135
expect(divA).toBe(divA2);
136
expect(divB).toBe(divB2);
137
});
138
+
139
+ itThrowsWhenRendering(
140
+ 'a suspending component outside a Suspense node',
141
+ async render => {
142
+ await render(
143
+ <div>
144
+ <React.Suspense />
145
+ <AsyncText text="Children" />
146
+ <React.Suspense />
147
+ </div>,
148
+ 1,
149
+ );
150
+ },
151
+ 'Add a <Suspense fallback=...> component higher in the tree',
152
+ );
153
+
154
+ itThrowsWhenRendering(
155
+ 'a suspending component without a Suspense above',
156
+ async render => {
157
+ await render(
158
+ <div>
159
+ <AsyncText text="Children" />
160
+ </div>,
161
+ 1,
162
+ );
163
+ },
164
+ 'Add a <Suspense fallback=...> component higher in the tree',
165
+ );
166
+
167
+ it('does not get confused by throwing null', () => {
168
+ function Bad() {
169
+ // eslint-disable-next-line no-throw-literal
170
+ throw null;
171
+ }
172
+
173
+ let didError;
174
+ let error;
175
+ try {
176
+ ReactDOMServer.renderToString(<Bad />);
177
+ } catch (err) {
178
+ didError = true;
179
+ error = err;
180
+ }
181
+ expect(didError).toBe(true);
182
+ expect(error).toBe(null);
183
+ });
184
+
185
+ it('does not get confused by throwing undefined', () => {
186
+ function Bad() {
187
+ // eslint-disable-next-line no-throw-literal
188
+ throw undefined;
189
+ }
190
+
191
+ let didError;
192
+ let error;
193
+ try {
194
+ ReactDOMServer.renderToString(<Bad />);
195
+ } catch (err) {
196
+ didError = true;
197
+ error = err;
198
+ }
199
+ expect(didError).toBe(true);
200
+ expect(error).toBe(undefined);
201
+ });
202
+
203
+ it('does not get confused by throwing a primitive', () => {
204
+ function Bad() {
205
+ // eslint-disable-next-line no-throw-literal
206
+ throw 'foo';
207
+ }
208
+
209
+ let didError;
210
+ let error;
211
+ try {
212
+ ReactDOMServer.renderToString(<Bad />);
213
+ } catch (err) {
214
+ didError = true;
215
+ error = err;
216
+ }
217
+ expect(didError).toBe(true);
218
+ expect(error).toBe('foo');
219
+ });
220
});
packages/react-dom/src/__tests__/ReactServerRendering-test.js
+64
@@ -705,6 +705,70 @@ describe('ReactDOMServer', () => {
705
}).toThrow('ReactDOMServer does not yet support lazy-loaded components.');
706
});
707
708
+ it('throws when suspending on the server', () => {
709
+ function AsyncFoo() {
710
+ throw new Promise(() => {});
711
+ }
712
+
713
+ expect(() => {
714
+ ReactDOMServer.renderToString(<AsyncFoo />);
715
+ }).toThrow('ReactDOMServer does not yet support Suspense.');
716
+ });
717
+
718
+ it('does not get confused by throwing null', () => {
719
+ function Bad() {
720
+ // eslint-disable-next-line no-throw-literal
721
+ throw null;
722
+ }
723
+
724
+ let didError;
725
+ let error;
726
+ try {
727
+ ReactDOMServer.renderToString(<Bad />);
728
+ } catch (err) {
729
+ didError = true;
730
+ error = err;
731
+ }
732
+ expect(didError).toBe(true);
733
+ expect(error).toBe(null);
734
+ });
735
+
736
+ it('does not get confused by throwing undefined', () => {
737
+ function Bad() {
738
+ // eslint-disable-next-line no-throw-literal
739
+ throw undefined;
740
+ }
741
+
742
+ let didError;
743
+ let error;
744
+ try {
745
+ ReactDOMServer.renderToString(<Bad />);
746
+ } catch (err) {
747
+ didError = true;
748
+ error = err;
749
+ }
750
+ expect(didError).toBe(true);
751
+ expect(error).toBe(undefined);
752
+ });
753
+
754
+ it('does not get confused by throwing a primitive', () => {
755
+ function Bad() {
756
+ // eslint-disable-next-line no-throw-literal
757
+ throw 'foo';
758
+ }
759
+
760
+ let didError;
761
+ let error;
762
+ try {
763
+ ReactDOMServer.renderToString(<Bad />);
764
+ } catch (err) {
765
+ didError = true;
766
+ error = err;
767
+ }
768
+ expect(didError).toBe(true);
769
+ expect(error).toBe('foo');
770
+ });
771
+
772
it('should throw (in dev) when children are mutated during render', () => {
773
function Wrapper(props) {
774
props.children[1] = <p key={1} />; // Mutation is illegal
packages/react-dom/src/server/ReactPartialRenderer.js
+16
-3
@@ -883,7 +883,8 @@ class ReactDOMServerRenderer {
883
const fallbackFrame = frame.fallbackFrame;
884
invariant(
885
fallbackFrame,
886
- 'suspense fallback not found, something is broken',
886
+ 'ReactDOMServer did not find an internal fallback frame for Suspense. ' +
887
+ 'This is a bug in React. Please file an issue.',
888
);
889
this.stack.push(fallbackFrame);
890
out[this.suspenseDepth] += '<!--$!-->';
@@ -909,8 +910,20 @@ class ReactDOMServerRenderer {
910
try {
911
outBuffer += this.render(child, frame.context, frame.domNamespace);
912
} catch (err) {
912
- if (enableSuspenseServerRenderer && typeof err.then === 'function') {
913
- suspended = true;
913
+ if (err != null && typeof err.then === 'function') {
914
+ if (enableSuspenseServerRenderer) {
915
+ invariant(
916
+ this.suspenseDepth > 0,
917
+ // TODO: include component name. This is a bit tricky with current factoring.
918
+ 'A React component suspended while rendering, but no fallback UI was specified.\n' +
919
+ '\n' +
920
+ 'Add a <Suspense fallback=...> component higher in the tree to ' +
921
+ 'provide a loading indicator or placeholder to display.',
922
+ );
923
+ suspended = true;
924
+ } else {
925
+ invariant(false, 'ReactDOMServer does not yet support Suspense.');
926
+ }
927
} else {
928
throw err;
929
}
scripts/error-codes/codes.json
+3
-2
@@ -301,7 +301,7 @@
301
"300": "Rendered fewer hooks than expected. This may be caused by an accidental early return statement.",
302
"301": "Too many re-renders. React limits the number of renders to prevent an infinite loop.",
303
"302": "It is not supported to run the profiling version of a renderer (for example, `react-dom/profiling`) without also replacing the `scheduler/tracing` module with `scheduler/tracing-profiling`. Your bundler might have a setting for aliasing both modules. Learn more at http://fb.me/react-profiling",
304
- "303": "suspense fallback not found, something is broken",
304
+ "303": "ReactDOMServer did not find an internal fallback frame for Suspense. This is a bug in React. Please file an issue.",
305
"304": "Maximum number of concurrent React renderers exceeded. This can happen if you are not properly destroying the Readable provided by React. Ensure that you call .destroy() on it if you no longer want to read from it, and did not read to the end. If you use .pipe() this should be automatic.",
306
"305": "The current renderer does not support hydration. This error is likely caused by a bug in React. Please file an issue.",
307
"306": "Element type is invalid. Received a promise that resolves to: %s. Lazy element type must resolve to a class or function.%s",
@@ -339,5 +339,6 @@
339
"338": "ReactDOMServer does not yet support the fundamental API.",
340
"339": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponder().",
341
"340": "Threw in newly mounted dehydrated component. This is likely a bug in React. Please file an issue.",
342
- "341": "We just came from a parent so we must have had a parent. This is a bug in React."
342
+ "341": "We just came from a parent so we must have had a parent. This is a bug in React.",
343
+ "342": "A React component suspended while rendering, but no fallback UI was specified.\n\nAdd a <Suspense fallback=...> component higher in the tree to provide a loading indicator or placeholder to display."
344
}