diff --git a/compat/src/suspense.js b/compat/src/suspense.js index 7b46716bfd..855d563e1d 100644 --- a/compat/src/suspense.js +++ b/compat/src/suspense.js @@ -22,7 +22,7 @@ options._catchError = function(error, newVNode, oldVNode) { oldCatchError(error, newVNode, oldVNode); }; -function detachedClone(vnode) { +function detachedClone(vnode, detachedParent, parentDom) { if (vnode) { if (vnode._component && vnode._component.__hooks) { vnode._component.__hooks._list.forEach(effect => { @@ -33,17 +33,39 @@ function detachedClone(vnode) { } vnode = assign({}, vnode); - vnode._component = null; - vnode._children = vnode._children && vnode._children.map(detachedClone); + if (vnode._component != null) { + if (vnode._component._parentDom === parentDom) { + vnode._component._parentDom = detachedParent; + } + vnode._component = null; + } + vnode._children = + vnode._children && + vnode._children.map(child => + detachedClone(child, detachedParent, parentDom) + ); } return vnode; } -function removeOriginal(vnode) { +function removeOriginal(vnode, detachedParent, originalParent) { if (vnode) { vnode._original = null; - vnode._children = vnode._children && vnode._children.map(removeOriginal); + vnode._children = + vnode._children && + vnode._children.map(child => + removeOriginal(child, detachedParent, originalParent) + ); + if (vnode._component) { + if (vnode._component._parentDom === detachedParent) { + if (vnode._dom) { + originalParent.insertBefore(vnode._dom, vnode._nextDom); + } + vnode._component._force = true; + vnode._component._parentDom = originalParent; + } + } } return vnode; } @@ -103,13 +125,32 @@ Suspense.prototype._childDidSuspend = function(promise, suspendingVNode) { suspendingComponent._suspendedComponentWillUnmount(); } }; + const _suspendedRender = suspendingComponent.render; + suspendingComponent.render = (props, state, context) => { + const result = _suspendedRender.call( + suspendingComponent, + props, + state, + context + ); + + suspendingComponent._renderCallbacks.push(onResolved); + + suspendingComponent.render = _suspendedRender; + return result; + }; const onSuspensionComplete = () => { if (!--c._pendingSuspensionCount) { // If the suspension was during hydration we don't need to restore the // suspended children into the _children array if (c.state._suspended) { - c._vnode._children[0] = removeOriginal(c.state._suspended); + const suspendedVNode = c.state._suspended; + c._vnode._children[0] = removeOriginal( + suspendedVNode, + suspendedVNode._component._parentDom, + suspendedVNode._component._originalParentDom + ); } c.setState({ _suspended: (c._detachOnNextRender = null) }); @@ -148,7 +189,13 @@ Suspense.prototype.render = function(props, state) { // (i.e. due to a setState further up in the tree) // it's _children prop is null, in this case we "forget" about the parked vnodes to detach if (this._vnode._children) { - this._vnode._children[0] = detachedClone(this._detachOnNextRender); + const detachedParent = document.createElement('div'); + const detachedComponent = this._vnode._children[0]._component; + this._vnode._children[0] = detachedClone( + this._detachOnNextRender, + detachedParent, + (detachedComponent._originalParentDom = detachedComponent._parentDom) + ); } this._detachOnNextRender = null; diff --git a/compat/test/browser/suspense.test.js b/compat/test/browser/suspense.test.js index 17e591401d..048747cca9 100644 --- a/compat/test/browser/suspense.test.js +++ b/compat/test/browser/suspense.test.js @@ -659,8 +659,7 @@ describe('suspense', () => { }); }); - // TODO: Fix this test - it.skip('should allow children to update state while suspending', () => { + it('should allow children to update state while suspending', () => { /** @type {(state: { s: string }) => void} */ let setState; class Stateful extends Component { @@ -1560,6 +1559,62 @@ describe('suspense', () => { expect(scratch.innerHTML).to.eql('
conditional hide
'); }); + it('should support updating state while suspended', async () => { + const [Suspender, suspend] = createSuspender(() =>
Suspender
); + + let increment; + + class Updater extends Component { + constructor(props) { + super(props); + this.state = { i: 0 }; + + increment = () => { + this.setState(({ i }) => ({ i: i + 1 })); + }; + } + + render(props, { i }) { + return ( +
+ i: {i} + +
+ ); + } + } + + render( + Suspended...}> + + , + scratch + ); + + expect(scratch.innerHTML).to.eql(`
i: 0
Suspender
`); + expect(Suspender.prototype.render).to.have.been.calledOnce; + + const [resolve] = suspend(); + rerender(); + + expect(scratch.innerHTML).to.eql(`
Suspended...
`); + + increment(); + rerender(); + + expect(scratch.innerHTML).to.eql(`
Suspended...
`); + + await resolve(() =>
Resolved
); + rerender(); + + expect(scratch.innerHTML).to.equal(`
i: 1
Resolved
`); + + increment(); + rerender(); + + expect(scratch.innerHTML).to.equal(`
i: 2
Resolved
`); + }); + it('should call componentWillUnmount on a suspended component', () => { const cWUSpy = sinon.spy(); @@ -1622,7 +1677,7 @@ describe('suspense', () => { expect(scratch.innerHTML).to.eql(`
conditional hide
`); }); - xit('should support sCU=false when un-suspending', () => { + it('should support sCU=false when un-suspending', () => { // See #2176 #2125 const [Suspender, suspend] = createSuspender(() =>
Hello
); @@ -1650,7 +1705,7 @@ describe('suspense', () => { }); }); - xit('should allow suspended children to update', () => { + it('should allow suspended children to update', () => { const log = []; class Logger extends Component { constructor(props) { @@ -1681,7 +1736,7 @@ describe('suspense', () => { throw this.state.promise; } - return 'hello'; + return
Suspender un-suspended
; } } @@ -1706,25 +1761,141 @@ describe('suspense', () => { rerender(); - /** - * These currently failing assertion shows the issue that we currently unmount - * the suspended tree (unlike react, which adds a display="none") and block any - * further processing on that tree. Thus updates below a suspended Suspense are - * getting lost. - */ expect(log).to.eql(['construct', 'render', 'render']); - - /** - * When the above assertion will hold true we will certainly run into the second issue - * here. The problem is that we do not remove suspensions from an instance of Suspense - * when one of its suspending children no longer throws because of a state - * update. - */ expect(scratch.innerHTML).to.eql( '
Suspender un-suspended
' ); }); + it('should allow multiple suspended children to update', () => { + function createSuspender() { + let suspender; + class Suspender extends Component { + constructor(props) { + super(props); + this.state = { promise: new Promise(() => {}) }; + suspender = this; + } + + unsuspend(content) { + this.setState({ promise: null, content }); + } + + render() { + if (this.state.promise) { + throw this.state.promise; + } + + return this.state.content; + } + } + return [content => suspender.unsuspend(content), Suspender]; + } + + const [unsuspender1, Suspender1] = createSuspender(); + const [unsuspender2, Suspender2] = createSuspender(); + + render( +
+ fallback}> + +
+ +
+
+
, + scratch + ); + + expect(scratch.innerHTML).to.eql('
'); + + // this rerender is needed because of Suspense issuing a forceUpdate itself + rerender(); + expect(scratch.innerHTML).to.eql('
fallback
'); + + unsuspender1( + <> +
Suspender un-suspended 1
+
Suspender un-suspended 2
+ + ); + + rerender(); + expect(scratch.innerHTML).to.eql('
fallback
'); + + unsuspender2(
Suspender 2
); + + rerender(); + expect(scratch.innerHTML).to.eql( + '
Suspender un-suspended 1
Suspender un-suspended 2
Suspender 2
' + ); + }); + + it('should allow suspended children children to update', () => { + function Suspender({ promise, content }) { + if (promise) { + throw promise; + } + return content; + } + + let parent; + class Parent extends Component { + constructor(props) { + super(props); + this.state = { promise: new Promise(() => {}), condition: true }; + parent = this; + } + + render() { + const { condition, promise, content } = this.state; + if (condition) { + return ; + } + return
Parent
; + } + } + + render( +
+ fallback}> + + +
, + scratch + ); + + expect(scratch.innerHTML).to.eql('
'); + + // this rerender is needed because of Suspense issuing a forceUpdate itself + rerender(); + expect(scratch.innerHTML).to.eql('
fallback
'); + + // hide the thus unsuspends + parent.setState({ condition: false }); + + rerender(); + expect(scratch.innerHTML).to.eql('
Parent
'); + + // show the thus re-suspends + parent.setState({ condition: true }); + rerender(); + + expect(scratch.innerHTML).to.eql('
fallback
'); + + // update state so that no longer suspends + parent.setState({ promise: null, content:
Content
}); + rerender(); + + expect(scratch.innerHTML).to.eql('
Content
'); + + // hide the again + parent.setState({ condition: false }); + rerender(); + + expect(scratch.innerHTML).to.eql('
Parent
'); + }); + it('should render delayed lazy components through components using shouldComponentUpdate', () => { const [Suspender1, suspend1] = createSuspender(() => 1); const [Suspender2, suspend2] = createSuspender(() => 2); diff --git a/mangle.json b/mangle.json index cee96f5899..b21de3aed7 100644 --- a/mangle.json +++ b/mangle.json @@ -61,6 +61,7 @@ "$_id": "__c", "$_contextRef": "__", "$_parentDom": "__P", + "$_originalParentDom": "__O", "$_prevState": "__u", "$_root": "__", "$_diff": "__b", @@ -72,7 +73,8 @@ "$_owner": "__o", "$_skipEffects": "__s", "$_rerenderCount": "__r", - "$_forwarded": "__f" + "$_forwarded": "__f", + "$_isSuspended": "__i" } } } \ No newline at end of file