Merge 6a84468 into 51f3e25

compat/src/index.js

@@ -12,7 +12,6 @@ const CAMEL_PROPS = /^(?:accent|alignment|arabic|baseline|cap|clip|color|fill|fl
 
 let oldEventHook = options.event;
 options.event = e => {
-	/* istanbul ignore next */
 	if (oldEventHook) e = oldEventHook(e);
 	e.persist = () => {};
 	return e.nativeEvent = e;
@@ -389,6 +388,10 @@ function setSafeDescriptor(proto, key) {
 		Object.defineProperty(proto, key, {
 			configurable: false,
 			get() { return this['UNSAFE_' + key]; },
+			// This `set` is only used if a user sets a lifecycle like cWU
+			// after setting a lifecycle like UNSAFE_cWU. I doubt anyone
+			// actually does this in practice so not testing it
+			/* istanbul ignore next */
 			set(v) { this['UNSAFE_' + key] = v; }
 		});
 	}
@@ -414,7 +417,6 @@ options.vnode = vnode => {
 		setSafeDescriptor(type.prototype, 'componentWillUpdate');
 		type._patchedLifecycles = true;
 	}
-	/* istanbul ignore next */
 	if (oldVNodeHook) oldVNodeHook(vnode);
 };
 

compat/test/browser/compat.test.js

@@ -21,9 +21,12 @@ describe('imported compat in preact', () => {
 		scratch.firstChild.click();
 
 		expect(spy).to.be.calledOnce;
-		expect(spy.args[0][0]).to.haveOwnProperty('persist');
-		expect(typeof spy.args[0][0].persist).to.equal('function');
-		expect(spy.args[0][0]).to.haveOwnProperty('nativeEvent');
+		const event = spy.args[0][0];
+		expect(event).to.haveOwnProperty('persist');
+		expect(event).to.haveOwnProperty('nativeEvent');
+		expect(typeof event.persist).to.equal('function');
+
+		expect(() => event.persist()).to.not.throw();
 	});
 
 	it('should normalize ondoubleclick event', () => {

compat/test/browser/forwardRef.test.js

@@ -239,6 +239,42 @@ describe('forwardRef', () => {
 		expect(renderCount).to.equal(3);
 	});
 
+	it('should bailout if forwardRef is wrapped in memo using function refs', () => {
+		const Component = props => <div ref={props.forwardedRef} />;
+
+		let renderCount = 0;
+
+		const App = memo(
+			forwardRef((props, ref) => {
+				renderCount++;
+				return <Component {...props} forwardedRef={ref} />;
+			}),
+		);
+
+		const ref = sinon.spy();
+
+		render(<App ref={ref} optional="foo" />, scratch);
+		expect(renderCount).to.equal(1);
+
+		expect(ref).to.have.been.called;
+
+		ref.resetHistory();
+		render(<App ref={ref} optional="foo" />, scratch);
+		expect(renderCount).to.equal(1);
+
+		const differentRef = sinon.spy();
+
+		render(<App ref={differentRef} optional="foo" />, scratch);
+		expect(renderCount).to.equal(2);
+
+		expect(ref).to.have.been.calledWith(null);
+		expect(differentRef).to.have.been.called;
+
+		differentRef.resetHistory();
+		render(<App ref={ref} optional="bar" />, scratch);
+		expect(renderCount).to.equal(3);
+	});
+
 	it('should pass ref through memo() with custom comparer function', () => {
 		const Foo = props => <div ref={props.forwardedRef} />;
 

compat/test/browser/memo.test.js

@@ -17,6 +17,7 @@ describe('memo()', () => {
 	});
 
 	it('should have isReactComponent flag', () => {
+		// eslint-disable-next-line react/display-name
 		let App = memo(() => <div>foo</div>);
 		expect(App.prototype.isReactComponent).to.equal(true);
 	});

compat/test/browser/options-compat.test.js

@@ -0,0 +1,79 @@
+import { vnodeSpy, eventSpy } from '../../../test/_util/optionSpies';
+import { render, createElement, Component, createRef } from '../../src';
+import { setupRerender } from 'preact/test-utils';
+import { setupScratch, teardown, createEvent } from '../../../test/_util/helpers';
+
+/** @jsx createElement */
+
+describe('compat options', () => {
+
+	/** @type {HTMLDivElement} */
+	let scratch;
+
+	/** @type {() => void} */
+	let rerender;
+
+	/** @type {() => void} */
+	let increment;
+
+	/** @type {import('../../src/index').PropRef<HTMLButtonElement | null>} */
+	let buttonRef;
+
+	beforeEach(() => {
+		scratch = setupScratch();
+		rerender = setupRerender();
+
+		vnodeSpy.resetHistory();
+		eventSpy.resetHistory();
+
+		buttonRef = createRef();
+	});
+
+	afterEach(() => {
+		teardown(scratch);
+	});
+
+	class ClassApp extends Component {
+		constructor() {
+			super();
+			this.state = { count: 0 };
+			increment = () =>
+				this.setState(({ count }) => ({
+					count: count + 1
+				}));
+		}
+
+		render() {
+			return (
+				<button ref={buttonRef} onClick={increment}>
+					{this.state.count}
+				</button>
+			);
+		}
+	}
+
+	it('should call old options on mount', () => {
+		render(<ClassApp />, scratch);
+
+		expect(vnodeSpy).to.have.been.called;
+	});
+
+	it('should call old options on event and update', () => {
+		render(<ClassApp />, scratch);
+		expect(scratch.innerHTML).to.equal('<button>0</button>');
+
+		buttonRef.current.dispatchEvent(createEvent('click'));
+		rerender();
+		expect(scratch.innerHTML).to.equal('<button>1</button>');
+
+		expect(vnodeSpy).to.have.been.called;
+		expect(eventSpy).to.have.been.called;
+	});
+
+	it('should call old options on unmount', () => {
+		render(<ClassApp />, scratch);
+		render(null, scratch);
+
+		expect(vnodeSpy).to.have.been.called;
+	});
+});

debug/src/debug.js

@@ -17,6 +17,8 @@ export function initDebug() {
 	let oldDiffed = options.diffed;
 	let oldVnode = options.vnode;
 	let oldCatchError = options._catchError;
+	let oldRoot = options._root;
+	let oldHook = options._hook;
 	const warnedComponents = { useEffect: {}, useLayoutEffect: {}, lazyPropTypes: {} };
 
 	options._catchError = (error, vnode, oldVNode) => {
@@ -58,6 +60,8 @@ export function initDebug() {
 			Expected a valid HTML node as a second argument to render.
 			Received ${parentNode} instead: render(<${vnode.type.name || vnode.type} />, ${parentNode});
 		`);
+
+		if (oldRoot) oldRoot(vnode, parentNode);
 	};
 
 	options._diff = vnode => {
@@ -164,6 +168,8 @@ export function initDebug() {
 		if (!comp) {
 			throw new Error('Hook can only be invoked from render methods.');
 		}
+
+		if (oldHook) oldHook(comp);
 	};
 
 	const warn = (property, err) => ({
@@ -221,34 +227,33 @@ export function initDebug() {
 
 		if (vnode._component && vnode._component.__hooks) {
 			let hooks = vnode._component.__hooks;
-			(hooks._list || []).forEach(hook => {
-				if (hook._callback && (!hook._args || !Array.isArray(hook._args))) {
-					/* istanbul ignore next */
-					console.warn(
-						`In ${vnode.type.name || vnode.type} you are calling useMemo/useCallback without passing arguments.\n` +
-						`This is a noop since it will not be able to memoize, it will execute it every render.`
-					);
-				}
-			});
-			if (hooks._pendingEffects && Array.isArray(hooks._pendingEffects)) {
+			if (Array.isArray(hooks._list)) {
+				hooks._list.forEach(hook => {
+					if (hook._callback && (!hook._args || !Array.isArray(hook._args))) {
+						console.warn(
+							`In ${getDisplayName(vnode)} you are calling useMemo/useCallback without passing arguments.\n` +
+							`This is a noop since it will not be able to memoize, it will execute it every render.`
+						);
+					}
+				});
+			}
+			if (Array.isArray(hooks._pendingEffects)) {
 				hooks._pendingEffects.forEach((effect) => {
 					if ((!effect._args || !Array.isArray(effect._args)) && !warnedComponents.useEffect[vnode.type]) {
 						warnedComponents.useEffect[vnode.type] = true;
-						/* istanbul ignore next */
 						console.warn('You should provide an array of arguments as the second argument to the "useEffect" hook.\n\n' +
 							'Not doing so will invoke this effect on every render.\n\n' +
-							'This effect can be found in the render of ' + (vnode.type.name || vnode.type) + '.');
+							'This effect can be found in the render of ' + getDisplayName(vnode) + '.');
 					}
 				});
 			}
-			if (hooks._pendingLayoutEffects && Array.isArray(hooks._pendingLayoutEffects)) {
+			if (Array.isArray(hooks._pendingLayoutEffects)) {
 				hooks._pendingLayoutEffects.forEach((layoutEffect) => {
 					if ((!layoutEffect._args || !Array.isArray(layoutEffect._args)) && !warnedComponents.useLayoutEffect[vnode.type]) {
 						warnedComponents.useLayoutEffect[vnode.type] = true;
-						/* istanbul ignore next */
 						console.warn('You should provide an array of arguments as the second argument to the "useLayoutEffect" hook.\n\n' +
 							'Not doing so will invoke this effect on every render.\n\n' +
-							'This effect can be found in the render of ' + (vnode.type.name || vnode.type) + '.');
+							'This effect can be found in the render of ' + getDisplayName(vnode) + '.');
 					}
 				});
 			}

debug/src/devtools/index.js

@@ -73,6 +73,7 @@ export function initDevTools() {
 		};
 
 		if (!hook._renderers) {
+			// eslint-disable-next-line no-console
 			console.info(
 				'Preact is not compatible with your version of react-devtools. We ' +
 				'will address this in future releases.'

debug/test/browser/debug.test.js

@@ -274,6 +274,7 @@ describe('debug', () => {
 		}
 
 		render(<Foo />, scratch);
+		forceUpdate();
 		expect(console.warn).to.not.be.called;
 
 		render(null, scratch);
@@ -477,6 +478,9 @@ describe('debug', () => {
 
 			expect(serializeVNode(<Foo foo={[1, 2, 3]} />))
 				.to.equal('<Foo foo="1,2,3" />');
+
+			expect(serializeVNode(<div prop={Object.create(null)} />))
+				.to.equal('<div prop="[object Object]" />');
 		});
 	});