From b35103a48292bd1bab25c5df368c1e87f15dde91 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 28 Jun 2023 11:29:59 +0200 Subject: [PATCH 1/7] queue refs up for applying after diffing complets --- src/component.js | 12 ++++++++++-- src/diff/children.js | 10 +++++----- src/diff/index.js | 15 ++++++++++----- src/render.js | 11 +++++++++-- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/component.js b/src/component.js index 13a69114e7..20d2a8f1e8 100644 --- a/src/component.js +++ b/src/component.js @@ -1,5 +1,5 @@ import { assign } from './util'; -import { diff, commitRoot } from './diff/index'; +import { diff, commitRoot, applyRef } from './diff/index'; import options from './options'; import { Fragment } from './create-element'; @@ -126,6 +126,7 @@ function renderComponent(component) { if (parentDom) { let commitQueue = []; + let refQueue = []; const oldVNode = assign({}, vnode); oldVNode._original = vnode._original + 1; @@ -138,8 +139,15 @@ function renderComponent(component) { vnode._hydrating != null ? [oldDom] : null, commitQueue, oldDom == null ? getDomSibling(vnode) : oldDom, - vnode._hydrating + vnode._hydrating, + refQueue ); + + refQueue.some(refs => { + for (let i = 0; i < refs.length; i++) { + applyRef(refs[i], refs[++i], refs[++i]); + } + }); commitRoot(commitQueue, vnode); if (vnode._dom != oldDom) { diff --git a/src/diff/children.js b/src/diff/children.js index 2a59a0cc2a..c84d1d0dfc 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -33,7 +33,8 @@ export function diffChildren( excessDomChildren, commitQueue, oldDom, - isHydrating + isHydrating, + refQueue ) { let i, j, @@ -138,7 +139,8 @@ export function diffChildren( excessDomChildren, commitQueue, oldDom, - isHydrating + isHydrating, + refQueue ); newDom = childVNode._dom; @@ -246,9 +248,7 @@ export function diffChildren( // Set refs only after unmount if (refs) { - for (i = 0; i < refs.length; i++) { - applyRef(refs[i], refs[++i], refs[++i]); - } + refQueue.push(refs); } } diff --git a/src/diff/index.js b/src/diff/index.js index 87c0ba21e4..4221e7de8a 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -31,7 +31,8 @@ export function diff( excessDomChildren, commitQueue, oldDom, - isHydrating + isHydrating, + refQueue ) { let tmp, newType = newVNode.type; @@ -239,7 +240,8 @@ export function diff( excessDomChildren, commitQueue, oldDom, - isHydrating + isHydrating, + refQueue ); c.base = newVNode._dom; @@ -269,7 +271,8 @@ export function diff( isSvg, excessDomChildren, commitQueue, - isHydrating + isHydrating, + refQueue ); } @@ -333,7 +336,8 @@ function diffElementNodes( isSvg, excessDomChildren, commitQueue, - isHydrating + isHydrating, + refQueue ) { let oldProps = oldVNode.props; let newProps = newVNode.props; @@ -445,7 +449,8 @@ function diffElementNodes( excessDomChildren ? excessDomChildren[0] : oldVNode._children && getDomSibling(oldVNode, 0), - isHydrating + isHydrating, + refQueue ); // Remove children that are not part of any vnode. diff --git a/src/render.js b/src/render.js index 3f92be680e..6336e2f681 100644 --- a/src/render.js +++ b/src/render.js @@ -1,5 +1,5 @@ import { EMPTY_OBJ } from './constants'; -import { commitRoot, diff } from './diff/index'; +import { applyRef, commitRoot, diff } from './diff/index'; import { createElement, Fragment } from './create-element'; import options from './options'; import { slice } from './util'; @@ -34,6 +34,7 @@ export function render(vnode, parentDom, replaceNode) { // List of effects that need to be called after diffing. let commitQueue = []; + let refQueue = []; diff( parentDom, // Determine the new vnode tree and store it on the DOM element on @@ -55,9 +56,15 @@ export function render(vnode, parentDom, replaceNode) { : oldVNode ? oldVNode._dom : parentDom.firstChild, - isHydrating + isHydrating, + refQueue ); + refQueue.some(refs => { + for (let i = 0; i < refs.length; i++) { + applyRef(refs[i], refs[++i], refs[++i]); + } + }); // Flush all queued effects commitRoot(commitQueue, vnode); } From e65cc819e5634929bad42ab8d6c949bbc69715fb Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 28 Jun 2023 11:39:17 +0200 Subject: [PATCH 2/7] execute setting ref to null immediately and queue up unsetting diff for post diff --- src/component.js | 8 ++-- src/diff/children.js | 8 +++- src/diff/index.js | 1 + src/render.js | 8 ++-- test/browser/refs.test.js | 94 ++++++++++++++++++++++++++++++++++++++- 5 files changed, 106 insertions(+), 13 deletions(-) diff --git a/src/component.js b/src/component.js index 20d2a8f1e8..b7fd4375f4 100644 --- a/src/component.js +++ b/src/component.js @@ -143,11 +143,9 @@ function renderComponent(component) { refQueue ); - refQueue.some(refs => { - for (let i = 0; i < refs.length; i++) { - applyRef(refs[i], refs[++i], refs[++i]); - } - }); + for (let i = 0; i < refQueue.length; i++) { + applyRef(refQueue[i], refQueue[++i], refQueue[++i]); + } commitRoot(commitQueue, vnode); if (vnode._dom != oldDom) { diff --git a/src/diff/children.js b/src/diff/children.js index c84d1d0dfc..ae46c64c5b 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -248,7 +248,13 @@ export function diffChildren( // Set refs only after unmount if (refs) { - refQueue.push(refs); + for (let i = 0; i < refs.length; i++) { + if (refs[i + 1]) { + refQueue.push(refs[i], refs[++i], refs[++i]); + } else { + applyRef(refs[i], refs[++i], refs[++i]); + } + } } } diff --git a/src/diff/index.js b/src/diff/index.js index 4221e7de8a..a00c9d1857 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -521,6 +521,7 @@ export function unmount(vnode, parentVNode, skipRemove) { if ((r = vnode.ref)) { if (!r.current || r.current === vnode._dom) { + console.log('applying', r, 'null'); applyRef(r, null, parentVNode); } } diff --git a/src/render.js b/src/render.js index 6336e2f681..3da4100bea 100644 --- a/src/render.js +++ b/src/render.js @@ -60,11 +60,9 @@ export function render(vnode, parentDom, replaceNode) { refQueue ); - refQueue.some(refs => { - for (let i = 0; i < refs.length; i++) { - applyRef(refs[i], refs[++i], refs[++i]); - } - }); + for (let i = 0; i < refQueue.length; i++) { + applyRef(refQueue[i], refQueue[++i], refQueue[++i]); + } // Flush all queued effects commitRoot(commitQueue, vnode); } diff --git a/test/browser/refs.test.js b/test/browser/refs.test.js index a5962ea500..f9f4144af7 100644 --- a/test/browser/refs.test.js +++ b/test/browser/refs.test.js @@ -1,5 +1,5 @@ import { setupRerender } from 'preact/test-utils'; -import { createElement, render, Component, createRef } from 'preact'; +import { createElement, render, Component, createRef, Fragment } from 'preact'; import { setupScratch, teardown } from '../_util/helpers'; /** @jsx createElement */ @@ -103,8 +103,8 @@ describe('refs', () => { 'called with H1', 'called with DIV', 'called with null', - 'called with H1', 'called with null', + 'called with H1', 'called with DIV' ]); }); @@ -543,4 +543,94 @@ describe('refs', () => { expect(calls.length).to.equal(1); expect(calls[0]).to.equal(scratch.firstChild.firstChild); }); + + // Test for #4049 + it('should first clean-up refs and after apply them', () => { + let calls = []; + let set; + class App extends Component { + constructor(props) { + super(props); + this.state = { + phase: 1 + }; + set = () => this.setState({ phase: 2 }); + } + + render(props, { phase }) { + return ( + + {phase === 1 ? ( +
+
+ r + ? calls.push('adding ref to two') + : calls.push('removing ref from two') + } + > + Element two +
+
+ r + ? calls.push('adding ref to three') + : calls.push('removing ref from three') + } + > + Element three +
+
+ ) : phase === 2 ? ( +
+
+ r + ? calls.push('adding ref to one') + : calls.push('removing ref from one') + } + > + Element one +
+
+
+ r + ? calls.push('adding ref to two') + : calls.push('removing ref from two') + } + > + Element two +
+
+ r + ? calls.push('adding ref to three') + : calls.push('removing ref from three') + } + > + Element three +
+
+
+ ) : null} + + ); + } + } + + render(, scratch); + + expect(calls).to.deep.equal(['adding ref to two', 'adding ref to three']); + calls = []; + + set(); + rerender(); + expect(calls).to.deep.equal([ + 'removing ref from two', + 'adding ref to two', + 'adding ref to three', + 'adding ref to one' + ]); + }); }); From 94228020d40d6f30a5baa11c453e79f48bd71c04 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 28 Jun 2023 11:46:58 +0200 Subject: [PATCH 3/7] save bytes --- src/component.js | 4 ++-- src/diff/children.js | 16 +++++++--------- src/diff/index.js | 1 - src/render.js | 4 ++-- test/browser/refs.test.js | 4 ++-- 5 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/component.js b/src/component.js index b7fd4375f4..b38d12e52d 100644 --- a/src/component.js +++ b/src/component.js @@ -125,8 +125,8 @@ function renderComponent(component) { parentDom = component._parentDom; if (parentDom) { - let commitQueue = []; - let refQueue = []; + let commitQueue = [], + refQueue = []; const oldVNode = assign({}, vnode); oldVNode._original = vnode._original + 1; diff --git a/src/diff/children.js b/src/diff/children.js index ae46c64c5b..f53e5d850b 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -146,9 +146,11 @@ export function diffChildren( newDom = childVNode._dom; if ((j = childVNode.ref) && oldVNode.ref != j) { - if (!refs) refs = []; - if (oldVNode.ref) refs.push(oldVNode.ref, null, childVNode); - refs.push(j, childVNode._component || newDom, childVNode); + if (oldVNode.ref) { + if (!refs) refs = []; + refs.push(oldVNode.ref, null, childVNode); + } + refQueue.push(j, childVNode._component || newDom, childVNode); } if (newDom != null) { @@ -248,12 +250,8 @@ export function diffChildren( // Set refs only after unmount if (refs) { - for (let i = 0; i < refs.length; i++) { - if (refs[i + 1]) { - refQueue.push(refs[i], refs[++i], refs[++i]); - } else { - applyRef(refs[i], refs[++i], refs[++i]); - } + for (i = 0; i < refs.length; i++) { + applyRef(refs[i], refs[++i], refs[++i]); } } } diff --git a/src/diff/index.js b/src/diff/index.js index a00c9d1857..4221e7de8a 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -521,7 +521,6 @@ export function unmount(vnode, parentVNode, skipRemove) { if ((r = vnode.ref)) { if (!r.current || r.current === vnode._dom) { - console.log('applying', r, 'null'); applyRef(r, null, parentVNode); } } diff --git a/src/render.js b/src/render.js index 3da4100bea..8dd8f344d2 100644 --- a/src/render.js +++ b/src/render.js @@ -33,8 +33,8 @@ export function render(vnode, parentDom, replaceNode) { createElement(Fragment, null, [vnode]); // List of effects that need to be called after diffing. - let commitQueue = []; - let refQueue = []; + let commitQueue = [], + refQueue = []; diff( parentDom, // Determine the new vnode tree and store it on the DOM element on diff --git a/test/browser/refs.test.js b/test/browser/refs.test.js index f9f4144af7..736b54b1de 100644 --- a/test/browser/refs.test.js +++ b/test/browser/refs.test.js @@ -628,9 +628,9 @@ describe('refs', () => { rerender(); expect(calls).to.deep.equal([ 'removing ref from two', + 'adding ref to one', 'adding ref to two', - 'adding ref to three', - 'adding ref to one' + 'adding ref to three' ]); }); }); From 44cf76531697a171779c8bf5e0fd8b9644141adf Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 28 Jun 2023 14:26:35 +0200 Subject: [PATCH 4/7] remove need for refs array --- src/diff/children.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/diff/children.js b/src/diff/children.js index f53e5d850b..3f1579835b 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -42,7 +42,6 @@ export function diffChildren( childVNode, newDom, firstChildDom, - refs, skew = 0; // This is a compression of oldParentVNode!=null && oldParentVNode != EMPTY_OBJ && oldParentVNode._children || EMPTY_ARR @@ -147,8 +146,7 @@ export function diffChildren( if ((j = childVNode.ref) && oldVNode.ref != j) { if (oldVNode.ref) { - if (!refs) refs = []; - refs.push(oldVNode.ref, null, childVNode); + applyRef(oldVNode.ref, null, childVNode); } refQueue.push(j, childVNode._component || newDom, childVNode); } @@ -247,13 +245,6 @@ export function diffChildren( unmount(oldChildren[i], oldChildren[i]); } } - - // Set refs only after unmount - if (refs) { - for (i = 0; i < refs.length; i++) { - applyRef(refs[i], refs[++i], refs[++i]); - } - } } function reorderChildren(childVNode, oldDom, parentDom) { From abc3e989fc98314256897cc077b193fd26f8d9c2 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 28 Jun 2023 14:30:28 +0200 Subject: [PATCH 5/7] save some more bytes --- src/component.js | 5 +---- src/diff/index.js | 6 +++++- src/render.js | 5 +---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/component.js b/src/component.js index b38d12e52d..4d333cf4f7 100644 --- a/src/component.js +++ b/src/component.js @@ -143,10 +143,7 @@ function renderComponent(component) { refQueue ); - for (let i = 0; i < refQueue.length; i++) { - applyRef(refQueue[i], refQueue[++i], refQueue[++i]); - } - commitRoot(commitQueue, vnode); + commitRoot(commitQueue, vnode, refQueue); if (vnode._dom != oldDom) { updateParentDomPointers(vnode); diff --git a/src/diff/index.js b/src/diff/index.js index 4221e7de8a..45df42bea2 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -296,7 +296,11 @@ export function diff( * which have callbacks to invoke in commitRoot * @param {import('../internal').VNode} root */ -export function commitRoot(commitQueue, root) { +export function commitRoot(commitQueue, root, refQueue) { + for (let i = 0; i < refQueue.length; i++) { + applyRef(refQueue[i], refQueue[++i], refQueue[++i]); + } + if (options._commit) options._commit(root, commitQueue); commitQueue.some(c => { diff --git a/src/render.js b/src/render.js index 8dd8f344d2..2a851be138 100644 --- a/src/render.js +++ b/src/render.js @@ -60,11 +60,8 @@ export function render(vnode, parentDom, replaceNode) { refQueue ); - for (let i = 0; i < refQueue.length; i++) { - applyRef(refQueue[i], refQueue[++i], refQueue[++i]); - } // Flush all queued effects - commitRoot(commitQueue, vnode); + commitRoot(commitQueue, vnode, refQueue); } /** From ac7ed378a2d3e38caecfb9dd5aa79d604810da38 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 28 Jun 2023 14:32:23 +0200 Subject: [PATCH 6/7] add types --- src/component.js | 2 +- src/diff/children.js | 1 + src/diff/index.js | 4 +++- src/render.js | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/component.js b/src/component.js index 4d333cf4f7..2270391120 100644 --- a/src/component.js +++ b/src/component.js @@ -1,5 +1,5 @@ import { assign } from './util'; -import { diff, commitRoot, applyRef } from './diff/index'; +import { diff, commitRoot } from './diff/index'; import options from './options'; import { Fragment } from './create-element'; diff --git a/src/diff/children.js b/src/diff/children.js index 3f1579835b..90afbaf65e 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -22,6 +22,7 @@ import { isArray } from '../util'; * render (except when hydrating). Can be a sibling DOM element when diffing * Fragments that have siblings. In most cases, it starts out as `oldChildren[0]._dom`. * @param {boolean} isHydrating Whether or not we are in hydration + * @param {Array} refQueue an array of elements needed to invoke refs */ export function diffChildren( parentDom, diff --git a/src/diff/index.js b/src/diff/index.js index 45df42bea2..2537a1b35b 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -20,7 +20,8 @@ import options from '../options'; * element any new dom elements should be placed around. Likely `null` on first * render (except when hydrating). Can be a sibling DOM element when diffing * Fragments that have siblings. In most cases, it starts out as `oldChildren[0]._dom`. - * @param {boolean} [isHydrating] Whether or not we are in hydration + * @param {boolean} isHydrating Whether or not we are in hydration + * @param {Array} refQueue an array of elements needed to invoke refs */ export function diff( parentDom, @@ -330,6 +331,7 @@ export function commitRoot(commitQueue, root, refQueue) { * @param {Array} commitQueue List of components * which have callbacks to invoke in commitRoot * @param {boolean} isHydrating Whether or not we are in hydration + * @param {Array} refQueue an array of elements needed to invoke refs * @returns {import('../internal').PreactElement} */ function diffElementNodes( diff --git a/src/render.js b/src/render.js index 2a851be138..02901b041d 100644 --- a/src/render.js +++ b/src/render.js @@ -1,5 +1,5 @@ import { EMPTY_OBJ } from './constants'; -import { applyRef, commitRoot, diff } from './diff/index'; +import { commitRoot, diff } from './diff/index'; import { createElement, Fragment } from './create-element'; import options from './options'; import { slice } from './util'; From 1f4e4643cce4f5642dc8286428880ec2eea26d23 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Thu, 29 Jun 2023 17:18:08 -0700 Subject: [PATCH 7/7] Add tests from #3860 --- hooks/test/browser/combinations.test.js | 79 ++++++++++++++++++++ hooks/test/browser/errorBoundary.test.js | 47 +++++++++++- hooks/test/browser/useLayoutEffect.test.js | 37 ++++++++++ test/browser/refs.test.js | 84 ++++++++++++++++++++++ 4 files changed, 246 insertions(+), 1 deletion(-) diff --git a/hooks/test/browser/combinations.test.js b/hooks/test/browser/combinations.test.js index 02db5b74f2..028110e8b9 100644 --- a/hooks/test/browser/combinations.test.js +++ b/hooks/test/browser/combinations.test.js @@ -398,4 +398,83 @@ describe('combinations', () => { }); expect(scratch.textContent).to.equal('2'); }); + + it.skip('parent and child refs should be set before all effects', () => { + const anchorId = 'anchor'; + const tooltipId = 'tooltip'; + const effectLog = []; + + let useRef2 = sinon.spy(init => { + const realRef = useRef(init); + const ref = useRef(init); + Object.defineProperty(ref, 'current', { + get: () => realRef.current, + set: value => { + realRef.current = value; + effectLog.push('set ref ' + value?.tagName); + } + }); + return ref; + }); + + function Tooltip({ anchorRef, children }) { + // For example, used to manually position the tooltip + const tooltipRef = useRef2(null); + + useLayoutEffect(() => { + expect(anchorRef.current?.id).to.equal(anchorId); + expect(tooltipRef.current?.id).to.equal(tooltipId); + effectLog.push('tooltip layout effect'); + }, [anchorRef, tooltipRef]); + useEffect(() => { + expect(anchorRef.current?.id).to.equal(anchorId); + expect(tooltipRef.current?.id).to.equal(tooltipId); + effectLog.push('tooltip effect'); + }, [anchorRef, tooltipRef]); + + return ( +
+
+ {children} +
+
+ ); + } + + function App() { + // For example, used to define what element to anchor the tooltip to + const anchorRef = useRef2(null); + + useLayoutEffect(() => { + expect(anchorRef.current?.id).to.equal(anchorId); + effectLog.push('anchor layout effect'); + }, [anchorRef]); + useEffect(() => { + expect(anchorRef.current?.id).to.equal(anchorId); + effectLog.push('anchor effect'); + }, [anchorRef]); + + return ( +
+

+ More info +

+ a tooltip +
+ ); + } + + act(() => { + render(, scratch); + }); + + expect(effectLog).to.deep.equal([ + 'set ref P', + 'set ref DIV', + 'tooltip layout effect', + 'anchor layout effect', + 'tooltip effect', + 'anchor effect' + ]); + }); }); diff --git a/hooks/test/browser/errorBoundary.test.js b/hooks/test/browser/errorBoundary.test.js index 9412ef421d..0e7de2f625 100644 --- a/hooks/test/browser/errorBoundary.test.js +++ b/hooks/test/browser/errorBoundary.test.js @@ -1,6 +1,6 @@ import { createElement, render } from 'preact'; import { setupScratch, teardown } from '../../../test/_util/helpers'; -import { useErrorBoundary } from 'preact/hooks'; +import { useErrorBoundary, useLayoutEffect } from 'preact/hooks'; import { setupRerender } from 'preact/test-utils'; /** @jsx createElement */ @@ -107,4 +107,49 @@ describe('errorBoundary', () => { expect(spy2).to.be.calledWith(error); expect(scratch.innerHTML).to.equal('

Error

'); }); + + it('does not invoke old effects when a cleanup callback throws an error and is handled', () => { + let throwErr = false; + let thrower = sinon.spy(() => { + if (throwErr) { + throw new Error('test'); + } + }); + let badEffect = sinon.spy(() => thrower); + let goodEffect = sinon.spy(); + + function EffectThrowsError() { + useLayoutEffect(badEffect); + return Test; + } + + function Child({ children }) { + useLayoutEffect(goodEffect); + return children; + } + + function App() { + const [err] = useErrorBoundary(); + return err ? ( +

Error

+ ) : ( + + + + ); + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('Test'); + expect(badEffect).to.be.calledOnce; + expect(goodEffect).to.be.calledOnce; + + throwErr = true; + render(, scratch); + rerender(); + expect(scratch.innerHTML).to.equal('

Error

'); + expect(thrower).to.be.calledOnce; + expect(badEffect).to.be.calledOnce; + expect(goodEffect).to.be.calledOnce; + }); }); diff --git a/hooks/test/browser/useLayoutEffect.test.js b/hooks/test/browser/useLayoutEffect.test.js index 5a5e75eb7b..f20fa1198a 100644 --- a/hooks/test/browser/useLayoutEffect.test.js +++ b/hooks/test/browser/useLayoutEffect.test.js @@ -476,4 +476,41 @@ describe('useLayoutEffect', () => { expect(calls.length).to.equal(1); expect(calls).to.deep.equal(['doing effecthi']); }); + + it('should run layout affects after all refs are invoked', () => { + const calls = []; + const verifyRef = name => el => { + calls.push(name); + expect(document.body.contains(el), name).to.equal(true); + }; + + const App = () => { + const ref = useRef(); + useLayoutEffect(() => { + expect(ref.current).to.equalNode(scratch.querySelector('p')); + + calls.push('doing effect'); + return () => { + calls.push('cleaning up'); + }; + }); + + return ( +
+

+ Hi +

+
+ ); + }; + + act(() => { + render(, scratch); + }); + expect(calls).to.deep.equal([ + 'callback ref inner', + 'callback ref outer', + 'doing effect' + ]); + }); }); diff --git a/test/browser/refs.test.js b/test/browser/refs.test.js index 736b54b1de..a69b846239 100644 --- a/test/browser/refs.test.js +++ b/test/browser/refs.test.js @@ -633,4 +633,88 @@ describe('refs', () => { 'adding ref to three' ]); }); + + it('should bind refs before componentDidMount', () => { + /** @type {import('preact').RefObject[]} */ + const refs = []; + + class Parent extends Component { + componentDidMount() { + // Child refs should be set + expect(refs.length).to.equal(2); + expect(refs[0].current.tagName).to.equal('SPAN'); + expect(refs[1].current.tagName).to.equal('SPAN'); + } + + render(props) { + return props.children; + } + } + + class Child extends Component { + constructor(props) { + super(props); + + this.ref = createRef(); + } + + componentDidMount() { + // SPAN refs should be set + expect(this.ref.current.tagName).to.equal('SPAN'); + expect(document.body.contains(this.ref.current)).to.equal(true); + refs.push(this.ref); + } + + render() { + return Hello; + } + } + + render( + + + + , + scratch + ); + + expect(scratch.innerHTML).to.equal('HelloHello'); + expect(refs[0].current).to.equalNode(scratch.firstChild); + expect(refs[1].current).to.equalNode(scratch.lastChild); + }); + + it('should call refs after element is added to document on initial mount', () => { + const verifyRef = name => el => + expect(document.body.contains(el), name).to.equal(true); + + function App() { + return ( +
+

Hello

+
+ ); + } + + render(, scratch); + }); + + it('should call refs after element is added to document on update', () => { + const verifyRef = name => el => + expect(document.body.contains(el), name).to.equal(true); + + function App({ show = false }) { + return ( +
+ {show && ( +

+ Hello +

+ )} +
+ ); + } + + render(, scratch); + render(, scratch); + }); });