Add tests from #3860

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 (
+				<div class="tooltip-wrapper">
+					<div id={tooltipId} ref={tooltipRef}>
+						{children}
+					</div>
+				</div>
+			);
+		}
+
+		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 (
+				<div>
+					<p id={anchorId} ref={anchorRef}>
+						More info
+					</p>
+					<Tooltip anchorRef={anchorRef}>a tooltip</Tooltip>
+				</div>
+			);
+		}
+
+		act(() => {
+			render(<App />, scratch);
+		});
+
+		expect(effectLog).to.deep.equal([
+			'set ref P',
+			'set ref DIV',
+			'tooltip layout effect',
+			'anchor layout effect',
+			'tooltip effect',
+			'anchor effect'
+		]);
+	});
 });

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('<p>Error</p>');
 	});
+
+	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 <span>Test</span>;
+		}
+
+		function Child({ children }) {
+			useLayoutEffect(goodEffect);
+			return children;
+		}
+
+		function App() {
+			const [err] = useErrorBoundary();
+			return err ? (
+				<p>Error</p>
+			) : (
+				<Child>
+					<EffectThrowsError />
+				</Child>
+			);
+		}
+
+		render(<App />, scratch);
+		expect(scratch.innerHTML).to.equal('<span>Test</span>');
+		expect(badEffect).to.be.calledOnce;
+		expect(goodEffect).to.be.calledOnce;
+
+		throwErr = true;
+		render(<App />, scratch);
+		rerender();
+		expect(scratch.innerHTML).to.equal('<p>Error</p>');
+		expect(thrower).to.be.calledOnce;
+		expect(badEffect).to.be.calledOnce;
+		expect(goodEffect).to.be.calledOnce;
+	});
 });

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 (
+				<div ref={verifyRef('callback ref outer')}>
+					<p ref={ref}>
+						<span ref={verifyRef('callback ref inner')}>Hi</span>
+					</p>
+				</div>
+			);
+		};
+
+		act(() => {
+			render(<App />, scratch);
+		});
+		expect(calls).to.deep.equal([
+			'callback ref inner',
+			'callback ref outer',
+			'doing effect'
+		]);
+	});
 });

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<HTMLSpanElement>[]} */
+		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 <span ref={this.ref}>Hello</span>;
+			}
+		}
+
+		render(
+			<Parent>
+				<Child />
+				<Child />
+			</Parent>,
+			scratch
+		);
+
+		expect(scratch.innerHTML).to.equal('<span>Hello</span><span>Hello</span>');
+		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 (
+				<div ref={verifyRef('div tag')}>
+					<p ref={verifyRef('p tag')}>Hello</p>
+				</div>
+			);
+		}
+
+		render(<App />, 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 (
+				<div>
+					{show && (
+						<p ref={verifyRef('p tag')}>
+							<span ref={verifyRef('inner span')}>Hello</span>
+						</p>
+					)}
+				</div>
+			);
+		}
+
+		render(<App />, scratch);
+		render(<App show />, scratch);
+	});
 });