From 0eac2f9d7d6b1a3850fbfa9418e47faa59175d93 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 11 Oct 2020 15:17:01 +0200 Subject: [PATCH 1/4] reset hooks state when suspense triggers --- compat/src/suspense.js | 3 ++ compat/test/browser/suspense.test.js | 53 +++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/compat/src/suspense.js b/compat/src/suspense.js index 87269d9734..c4133347e8 100644 --- a/compat/src/suspense.js +++ b/compat/src/suspense.js @@ -24,6 +24,9 @@ options._catchError = function(error, newVNode, oldVNode) { function detachedClone(vnode) { if (vnode) { + if (vnode._component) { + vnode._component.__hooks = null; + } vnode = assign({}, vnode); vnode._component = null; vnode._children = vnode._children && vnode._children.map(detachedClone); diff --git a/compat/test/browser/suspense.test.js b/compat/test/browser/suspense.test.js index da3a8bf12d..2419d23bc0 100644 --- a/compat/test/browser/suspense.test.js +++ b/compat/test/browser/suspense.test.js @@ -6,7 +6,8 @@ import React, { Suspense, lazy, Fragment, - createContext + createContext, + useState } from 'preact/compat'; import { setupScratch, teardown } from '../../../test/_util/helpers'; @@ -161,6 +162,56 @@ describe('suspense', () => { }); }); + it('should reset hooks of components', () => { + let set; + const LazyComp = ({ name }) =>
Hello from {name}
; + + /** @type {() => Promise} */ + let resolve; + const Lazy = lazy(() => { + const p = new Promise(res => { + resolve = () => { + res({ default: LazyComp }); + return p; + }; + }); + + return p; + }); + + const Parent = ({ children }) => { + const [state, setState] = useState(false); + set = setState; + + return ( +
+

hi

+ {state && children} +
+ ); + }; + + render( + Suspended...}> + + + + , + scratch + ); + expect(scratch.innerHTML).to.eql(`

hi

`); + + set(true); + rerender(); + + expect(scratch.innerHTML).to.eql('
Suspended...
'); + + return resolve().then(() => { + rerender(); + expect(scratch.innerHTML).to.eql(`

hi

`); + }); + }); + it('should support a call to setState before rendering the fallback', () => { const LazyComp = ({ name }) =>
Hello from {name}
; From dad1281909926c69ec2c802866ede3c54d32dba8 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 11 Oct 2020 15:21:17 +0200 Subject: [PATCH 2/4] cleanup effects --- compat/src/suspense.js | 6 ++++++ mangle.json | 1 + 2 files changed, 7 insertions(+) diff --git a/compat/src/suspense.js b/compat/src/suspense.js index c4133347e8..e361eef84e 100644 --- a/compat/src/suspense.js +++ b/compat/src/suspense.js @@ -25,6 +25,12 @@ options._catchError = function(error, newVNode, oldVNode) { function detachedClone(vnode) { if (vnode) { if (vnode._component) { + if (vnode._component.__hooks._pendingEffects.length) { + vnode._component.__hooks._pendingEffects.forEach(effect => { + if (typeof effect._cleanup == 'function') effect._cleanup(); + }); + } + vnode._component.__hooks = null; } vnode = assign({}, vnode); diff --git a/mangle.json b/mangle.json index 1a9c680ec8..212298ed38 100644 --- a/mangle.json +++ b/mangle.json @@ -25,6 +25,7 @@ "props": { "cname": 6, "props": { + "$_cleanup": "__c", "$_afterPaintQueued": "__a", "$__hooks": "__H", "$_list": "__", From 0c1182d7ab99283cb2284e35dd4e2d1327484e05 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 11 Oct 2020 15:41:56 +0200 Subject: [PATCH 3/4] correctly cleanup effects --- compat/src/suspense.js | 13 ++++-- compat/test/browser/suspense.test.js | 70 +++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/compat/src/suspense.js b/compat/src/suspense.js index e361eef84e..f75ddbb07e 100644 --- a/compat/src/suspense.js +++ b/compat/src/suspense.js @@ -24,19 +24,24 @@ options._catchError = function(error, newVNode, oldVNode) { function detachedClone(vnode) { if (vnode) { - if (vnode._component) { - if (vnode._component.__hooks._pendingEffects.length) { - vnode._component.__hooks._pendingEffects.forEach(effect => { + if (vnode._component && vnode._component.__hooks) { + if (vnode._component.__hooks._list.length) { + vnode._component.__hooks._list.forEach(effect => { if (typeof effect._cleanup == 'function') effect._cleanup(); }); } - vnode._component.__hooks = null; + vnode._component.__hooks = { + _list: [], + _pendingEffects: [] + }; } + vnode = assign({}, vnode); vnode._component = null; vnode._children = vnode._children && vnode._children.map(detachedClone); } + return vnode; } diff --git a/compat/test/browser/suspense.test.js b/compat/test/browser/suspense.test.js index 2419d23bc0..9576abd86d 100644 --- a/compat/test/browser/suspense.test.js +++ b/compat/test/browser/suspense.test.js @@ -7,7 +7,9 @@ import React, { lazy, Fragment, createContext, - useState + useState, + useEffect, + useLayoutEffect } from 'preact/compat'; import { setupScratch, teardown } from '../../../test/_util/helpers'; @@ -212,6 +214,72 @@ describe('suspense', () => { }); }); + it('should call effect cleanups', () => { + let set; + const effectSpy = sinon.spy(); + const layoutEffectSpy = sinon.spy(); + const LazyComp = ({ name }) =>
Hello from {name}
; + + /** @type {() => Promise} */ + let resolve; + const Lazy = lazy(() => { + const p = new Promise(res => { + resolve = () => { + res({ default: LazyComp }); + return p; + }; + }); + + return p; + }); + + const Parent = ({ children }) => { + const [state, setState] = useState(false); + set = setState; + useEffect(() => { + return () => { + effectSpy(); + }; + }, []); + + useLayoutEffect(() => { + return () => { + layoutEffectSpy(); + }; + }, []); + + return state ? ( +
{children}
+ ) : ( +
+

hi

+
+ ); + }; + + render( + Suspended...}> + + + + , + scratch + ); + + set(true); + rerender(); + expect(scratch.innerHTML).to.eql('
Suspended...
'); + expect(effectSpy).to.be.calledOnce; + expect(layoutEffectSpy).to.be.calledOnce; + + return resolve().then(() => { + rerender(); + expect(effectSpy).to.be.calledOnce; + expect(layoutEffectSpy).to.be.calledOnce; + expect(scratch.innerHTML).to.eql(`

hi

`); + }); + }); + it('should support a call to setState before rendering the fallback', () => { const LazyComp = ({ name }) =>
Hello from {name}
; From a67979531d8467f9da1d4a3f597f45c937d0f889 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 11 Oct 2020 15:52:57 +0200 Subject: [PATCH 4/4] golf some bytes --- compat/src/suspense.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/compat/src/suspense.js b/compat/src/suspense.js index f75ddbb07e..f357ee8941 100644 --- a/compat/src/suspense.js +++ b/compat/src/suspense.js @@ -25,16 +25,11 @@ options._catchError = function(error, newVNode, oldVNode) { function detachedClone(vnode) { if (vnode) { if (vnode._component && vnode._component.__hooks) { - if (vnode._component.__hooks._list.length) { - vnode._component.__hooks._list.forEach(effect => { - if (typeof effect._cleanup == 'function') effect._cleanup(); - }); - } + vnode._component.__hooks._list.forEach(effect => { + if (typeof effect._cleanup == 'function') effect._cleanup(); + }); - vnode._component.__hooks = { - _list: [], - _pendingEffects: [] - }; + vnode._component.__hooks = null; } vnode = assign({}, vnode);