Skip to content

Commit d781aa5

Browse files
committed
Fix hydration-susepsnse crashing due to sCU bail
1 parent 65b32cb commit d781aa5

2 files changed

Lines changed: 36 additions & 52 deletions

File tree

compat/test/browser/suspense-hydration.test.jsx

Lines changed: 27 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -290,62 +290,47 @@ describe('suspense hydration', () => {
290290
});
291291
});
292292

293-
it('Should not crash when oldVNode._children is null during shouldComponentUpdate optimization', () => {
294-
const originalHtml = '<div>Hello</div>';
295-
scratch.innerHTML = originalHtml;
293+
it('does not crash when a hydrated suspended component bails out with shouldComponentUpdate', () => {
294+
scratch.innerHTML = '<div>ssr</div>';
296295
clearLog();
297296

298-
class ErrorBoundary extends React.Component {
299-
constructor(props) {
300-
super(props);
301-
this.state = { hasError: false };
302-
}
297+
const promise = new Promise(() => {});
298+
let update;
303299

304-
static getDerivedStateFromError() {
305-
return { hasError: true };
300+
class Suspender extends React.Component {
301+
shouldComponentUpdate() {
302+
return false;
306303
}
307304

308305
render() {
309-
return this.props.children;
306+
throw promise;
310307
}
311308
}
312309

313-
const [Lazy, resolve] = createLazy();
314-
function App() {
315-
return (
316-
<Suspense>
317-
<ErrorBoundary>
318-
<Lazy />
319-
</ErrorBoundary>
320-
</Suspense>
321-
);
322-
}
323-
324-
hydrate(<App />, scratch);
325-
rerender(); // Flush rerender queue to mimic what preact will really do
326-
expect(scratch.innerHTML).to.equal(originalHtml);
327-
expect(getLog()).to.deep.equal([]);
328-
clearLog();
329-
330-
let i = 0;
331-
class ThrowOrRender extends React.Component {
332-
shouldComponentUpdate() {
333-
return i === 0;
310+
class App extends React.Component {
311+
constructor(props) {
312+
super(props);
313+
this.state = { tick: 0 };
314+
update = () => this.setState({ tick: this.state.tick + 1 });
334315
}
316+
335317
render() {
336-
if (i === 0) {
337-
i++;
338-
throw new Error('Test error');
339-
}
340-
return <div>Hello</div>;
318+
return (
319+
<Suspense fallback={<div>loading</div>}>
320+
<Suspender tick={this.state.tick} />
321+
</Suspense>
322+
);
341323
}
342324
}
343325

344-
return resolve(ThrowOrRender).then(() => {
345-
rerender();
346-
expect(scratch.innerHTML).to.equal(originalHtml);
347-
clearLog();
348-
});
326+
hydrate(<App />, scratch);
327+
expect(scratch.innerHTML).to.equal('<div>ssr</div>');
328+
329+
update();
330+
expect(() => rerender()).not.to.throw();
331+
expect(scratch.innerHTML).to.equal('<div>ssr</div>');
332+
expect(getLog()).to.deep.equal([]);
333+
clearLog();
349334
});
350335

351336
it('should leave DOM untouched when suspending while hydrating', () => {

src/diff/index.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -303,21 +303,20 @@ export function diff(
303303
excessDomChildren[excessDomChildren.indexOf(oldDom)] = NULL;
304304
}
305305
newVNode._dom = oldDom;
306-
} else {
307-
if (excessDomChildren != NULL) {
308-
for (let i = excessDomChildren.length; i--; ) {
309-
removeNode(excessDomChildren[i]);
310-
}
306+
} else if (excessDomChildren != NULL) {
307+
for (let i = excessDomChildren.length; i--; ) {
308+
removeNode(excessDomChildren[i]);
311309
}
312-
markAsForce(newVNode);
313310
}
314311
} else {
315312
newVNode._dom = oldVNode._dom;
316-
if (!newVNode._children && oldVNode._children) {
317-
newVNode._children = oldVNode._children;
318-
}
319-
if (!e.then) markAsForce(newVNode);
320313
}
314+
315+
if (newVNode._children == NULL) {
316+
newVNode._children = oldVNode._children || [];
317+
}
318+
319+
if (!e.then) markAsForce(newVNode);
321320
options._catchError(e, newVNode, oldVNode);
322321
}
323322
} else if (

0 commit comments

Comments
 (0)