diff --git a/compat/src/suspense.js b/compat/src/suspense.js index 6d5333a3b7..8a55b0971e 100644 --- a/compat/src/suspense.js +++ b/compat/src/suspense.js @@ -27,6 +27,7 @@ const oldUnmount = options.unmount; options.unmount = function (vnode) { /** @type {import('./internal').Component} */ const component = vnode._component; + if (component) component._unmounted = true; if (component && component._onResolve) { component._onResolve(); } @@ -129,7 +130,7 @@ Suspense.prototype._childDidSuspend = function (promise, suspendingVNode) { let resolved = false; const onResolved = () => { - if (resolved) return; + if (resolved || c._unmounted) return; resolved = true; suspendingComponent._onResolve = null; @@ -236,6 +237,7 @@ Suspense.prototype.render = function (props, state) { * @returns {((unsuspend: () => void) => void)?} */ export function suspended(vnode) { + if (!vnode._parent) return null; /** @type {import('./internal').Component} */ let component = vnode._parent._component; return component && component._suspended && component._suspended(vnode); diff --git a/compat/test/browser/suspense.test.js b/compat/test/browser/suspense.test.js index 2998d80f30..9eaaf326fb 100644 --- a/compat/test/browser/suspense.test.js +++ b/compat/test/browser/suspense.test.js @@ -14,6 +14,7 @@ import React, { } from 'preact/compat'; import { setupScratch, teardown } from '../../../test/_util/helpers'; import { createLazy, createSuspender } from './suspense-utils'; +import { expect } from 'chai'; const h = React.createElement; /* eslint-env browser, mocha */ @@ -2188,6 +2189,282 @@ describe('suspense', () => { }); }); + it('should not crash when suspended child updates after unmount', () => { + let childInstance = null; + const neverResolvingPromise = new Promise(() => {}); + + class ThrowingChild extends Component { + constructor(props) { + super(props); + this.state = { suspend: false, value: 0 }; + childInstance = this; + } + + render(props, state) { + if (state.suspend) { + throw neverResolvingPromise; + } + return
value:{state.value}
; + } + } + + render( + Suspended...}> + + , + scratch + ); + + expect(scratch.innerHTML).to.equal('
value:0
'); + + childInstance.setState({ suspend: true }); + rerender(); + expect(scratch.innerHTML).to.equal('
Suspended...
'); + + render(null, scratch); + expect(scratch.innerHTML).to.equal(''); + + childInstance.setState({ value: 1 }); + rerender(); + + expect(scratch.innerHTML).to.equal(''); + }); + + it('should not crash when suspended child updates after diffed unmount', () => { + let childInstance = null; + const neverResolvingPromise = new Promise(() => {}); + + class ThrowingChild extends Component { + constructor(props) { + super(props); + this.state = { suspend: false, value: 0 }; + childInstance = this; + } + + render(props, state) { + if (state.suspend) { + throw neverResolvingPromise; + } + return
value:{state.value}
; + } + } + + const HelloWorld = () =>

Hello world

; + + let set; + const App = () => { + const [show, setShow] = useState(true); + set = setShow; + return show ? ( + Suspended...}> + + + ) : ( + + ); + }; + + render(, scratch); + + expect(scratch.innerHTML).to.equal('
value:0
'); + + childInstance.setState({ suspend: true }); + rerender(); + expect(scratch.innerHTML).to.equal('
Suspended...
'); + + set(false); + rerender(); + expect(scratch.innerHTML).to.equal('

Hello world

'); + + childInstance.setState({ value: 1 }); + rerender(); + + expect(scratch.innerHTML).to.equal('

Hello world

'); + }); + + it('should not crash when suspended child resolves after unmount', async () => { + let childInstance = null, + res; + const neverResolvingPromise = new Promise(r => { + res = r; + }); + + class ThrowingChild extends Component { + constructor(props) { + super(props); + this.state = { suspend: false, value: 0 }; + childInstance = this; + } + + render(props, state) { + if (state.suspend) { + throw neverResolvingPromise; + } + return
value:{state.value}
; + } + } + + render( + Suspended...}> + + , + scratch + ); + + expect(scratch.innerHTML).to.equal('
value:0
'); + + childInstance.setState({ suspend: true }); + rerender(); + expect(scratch.innerHTML).to.equal('
Suspended...
'); + + render(null, scratch); + expect(scratch.innerHTML).to.equal(''); + + res(); + return neverResolvingPromise.then(() => { + rerender(); + expect(scratch.innerHTML).to.equal(''); + }); + }); + + it('should not crash when suspended child resolves after diffed unmount', async () => { + let childInstance = null, + res; + const neverResolvingPromise = new Promise(r => { + res = r; + }); + + class ThrowingChild extends Component { + constructor(props) { + super(props); + this.state = { suspend: false, value: 0 }; + childInstance = this; + } + + render(props, state) { + if (state.suspend) { + throw neverResolvingPromise; + } + return
value:{state.value}
; + } + } + + const HelloWorld = () =>

Hello world

; + + let set; + const App = () => { + const [show, setShow] = useState(true); + set = setShow; + return show ? ( + Suspended...}> + + + ) : ( + + ); + }; + + render(, scratch); + + expect(scratch.innerHTML).to.equal('
value:0
'); + + childInstance.setState({ suspend: true }); + rerender(); + expect(scratch.innerHTML).to.equal('
Suspended...
'); + + set(false); + rerender(); + expect(scratch.innerHTML).to.equal('

Hello world

'); + + res(); + return neverResolvingPromise.then(() => { + rerender(); + expect(scratch.innerHTML).to.equal('

Hello world

'); + }); + }); + + it('should not crash when suspense promise resolves after unmount', () => { + let resolve; + const promise = new Promise(r => { + resolve = r; + }); + + class ThrowingChild extends Component { + render() { + throw promise; + } + } + + render( + Suspended...}> + + , + scratch + ); + rerender(); + + expect(scratch.innerHTML).to.equal('
Suspended...
'); + + render(null, scratch); + expect(scratch.innerHTML).to.equal(''); + + resolve(); + + return promise.then(() => { + rerender(); + expect(scratch.innerHTML).to.equal(''); + }); + }); + + it('should not crash when useContext is used in a suspending component', () => { + const TestContext = createContext('default'); + let resolve; + let shouldSuspend = false; + const promise = new Promise(r => { + resolve = r; + }); + + function ContextUser() { + const value = React.useContext(TestContext); + if (shouldSuspend) { + throw promise; + } + return
Context: {value}
; + } + + render( + + Suspended...}> + + + , + scratch + ); + + expect(scratch.innerHTML).to.equal('
Context: test-value
'); + + shouldSuspend = true; + render( + + Suspended...}> + + + , + scratch + ); + rerender(); + + expect(scratch.innerHTML).to.equal('
Suspended...
'); + + shouldSuspend = false; + resolve(); + + return promise.then(() => { + rerender(); + expect(scratch.innerHTML).to.equal('
Context: test-value
'); + }); + }); + it('should not crash if fallback has same DOM as suspended nodes', () => { const [Lazy, resolveLazy] = createLazy(); diff --git a/mangle.json b/mangle.json index 2d15afef53..0de7512031 100644 --- a/mangle.json +++ b/mangle.json @@ -49,6 +49,7 @@ "$_children": "__k", "$_pendingSuspensionCount": "__u", "$_childDidSuspend": "__c", + "$_unmounted": "__z", "$_onResolve": "__R", "$_suspended": "__a", "$_dom": "__e",