Merge 232d896 into 984f18d

compat/mangle.json

@@ -23,7 +23,6 @@
       "$_render": "__r",
       "$_hook": "__h",
       "$_catchError": "__e",
-      "$_catchRender": "__E",
       "$_unmount": "_e"
     }
   }

compat/src/index.js

@@ -1,6 +1,6 @@
 import { hydrate, render as preactRender, cloneElement as preactCloneElement, createRef, h, Component, options, toChildArray, createContext, Fragment } from 'preact';
 import * as hooks from 'preact/hooks';
-import { Suspense, lazy, catchRender } from './suspense';
+import { Suspense, lazy } from './suspense';
 import { assign, removeNode } from '../../src/util';
 
 const version = '16.8.0'; // trick libraries to think we are react
@@ -18,11 +18,6 @@ options.event = e => {
 	return e.nativeEvent = e;
 };
 
-let oldCatchRender = options._catchRender;
-options._catchRender = (error, newVNode, oldVNode) => (
-	oldCatchRender && oldCatchRender(error, newVNode, oldVNode) || catchRender(error, newVNode, oldVNode)
-);
-
 /**
  * Legacy version of createElement.
  * @param {import('./internal').VNode["type"]} type The node name or Component constructor

compat/src/suspense.js

@@ -1,34 +1,29 @@
-import { Component, createElement, _unmount as unmount } from 'preact';
+import { Component, createElement, _unmount as unmount, options } from 'preact';
 import { removeNode } from '../../src/util';
 
-/**
- * @param {any} error
- * @param {import('./internal').VNode} newVNode
- * @param {import('./internal').VNode} oldVNode
- */
-export function catchRender(error, newVNode, oldVNode) {
-	// thrown Promises are meant to suspend...
-	if (error.then) {
+const oldCatchError = options._catchError;
+options._catchError = function (error, newVNode, oldVNode) {
+	if (error.then && oldVNode) {
 
 		/** @type {import('./internal').Component} */
 		let component;
 		let vnode = newVNode;
 
-		for (; vnode; vnode = vnode._parent) {
+		for (; vnode = vnode._parent;) {
 			if ((component = vnode._component) && component._childDidSuspend) {
 				if (oldVNode) {
 					newVNode._dom = oldVNode._dom;
 					newVNode._children = oldVNode._children;
 				}
 
 				component._childDidSuspend(error);
-				return true;
+				return; // Don't call oldCatchError if we found a Suspense
 			}
 		}
 	}
 
-	return false;
-}
+	oldCatchError(error, newVNode, oldVNode);
+};
 
 function detachDom(children) {
 	for (let i = 0; i < children.length; i++) {

debug/mangle.json

@@ -35,7 +35,6 @@
       "$_render": "__r",
       "$_hook": "__h",
       "$_catchError": "__e",
-      "$_catchRender": "__E",
       "$_unmount": "_e"
     }
   }

debug/src/debug.js

@@ -16,13 +16,25 @@ export function initDebug() {
 	let oldBeforeDiff = options._diff;
 	let oldDiffed = options.diffed;
 	let oldVnode = options.vnode;
+	let oldCatchError = options._catchError;
 	const warnedComponents = { useEffect: {}, useLayoutEffect: {}, lazyPropTypes: {} };
 
-	options._catchError = (error, vnode) => {
+	options._catchError = (error, vnode, oldVNode) => {
 		let component = vnode && vnode._component;
 		if (component && typeof error.then === 'function') {
-			error = new Error('Missing Suspense. The throwing component was: ' + (component.displayName || component.name));
+			const promise = error;
+			error = new Error('Missing Suspense. The throwing component was: ' + getDisplayName(vnode));
+
+			let parent = vnode;
+			for (; parent; parent = parent._parent) {
+				if (parent._component && parent._component._childDidSuspend) {
+					error = promise;
+					break;
+				}
+			}
 		}
+
+		oldCatchError(error, vnode, oldVNode);
 	};
 
 	options._root = (vnode, parentNode) => {

debug/test/browser/debug.test.js

@@ -268,6 +268,20 @@ describe('debug', () => {
 		expect(console.error).to.not.be.called;
 	});
 
+	it('should throw an error when missing Suspense', () => {
+		const Foo = () => <div>Foo</div>;
+		const LazyComp = lazy(() => new Promise(resolve => resolve({ default: Foo })));
+		const fn = () => {
+			render((
+				<Fragment>
+					<LazyComp />
+				</Fragment>
+			), scratch);
+		};
+
+		expect(fn).to.throw(/Missing Suspense/gi);
+	});
+
 	describe('duplicate keys', () => {
 		const List = props => <ul>{props.children}</ul>;
 		const ListItem = props => <li>{props.children}</li>;

hooks/mangle.json

@@ -17,7 +17,6 @@
       "$_render": "__r",
       "$_hook": "__h",
       "$_catchError": "__e",
-      "$_catchRender": "__E",
       "$_unmount": "_e"
     }
   }

mangle.json

@@ -39,7 +39,6 @@
       "$_render": "__r",
       "$_hook": "__h",
       "$_catchError": "__e",
-      "$_catchRender": "__E",
       "$_unmount": "_e"
     }
   }

src/diff/children.js

@@ -85,7 +85,7 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
 			newDom = diff(parentDom, childVNode, oldVNode, context, isSvg, excessDomChildren, mounts, null, oldDom);
 
 			if ((j = childVNode.ref) && oldVNode.ref != j) {
-				(refs || (refs=[])).push(j, childVNode._component || newDom);
+				(refs || (refs=[])).push(j, childVNode._component || newDom, childVNode);
 			}
 
 			// Only proceed if the vnode has not been unmounted by `diff()` above.
@@ -143,12 +143,12 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
 	if (excessDomChildren!=null && typeof newParentVNode.type !== 'function') for (i=excessDomChildren.length; i--; ) if (excessDomChildren[i]!=null) removeNode(excessDomChildren[i]);
 
 	// Remove remaining oldChildren if there are any.
-	for (i=oldChildrenLength; i--; ) if (oldChildren[i]!=null) unmount(oldChildren[i], newParentVNode);
+	for (i=oldChildrenLength; i--; ) if (oldChildren[i]!=null) unmount(oldChildren[i], oldChildren[i]);
 
 	// Set refs only after unmount
 	if (refs) {
 		for (i = 0; i < refs.length; i++) {
-			applyRef(refs[i], refs[++i], newParentVNode);
+			applyRef(refs[i], refs[++i], refs[++i]);
 		}
 	}
 }

src/diff/index.js

@@ -112,15 +112,9 @@ export function diff(parentDom, newVNode, oldVNode, context, isSvg, excessDomChi
 			c._vnode = newVNode;
 			c._parentDom = parentDom;
 
-			try {
-				tmp = c.render(c.props, c.state, c.context);
-				let isTopLevelFragment = tmp != null && tmp.type == Fragment && tmp.key == null;
-				toChildArray(isTopLevelFragment ? tmp.props.children : tmp, newVNode._children=[], coerceToVNode, true);
-			}
-			catch (e) {
-				if ((tmp = options._catchRender) && tmp(e, newVNode, oldVNode)) break outer;
-				throw e;
-			}
+			tmp = c.render(c.props, c.state, c.context);
+			let isTopLevelFragment = tmp != null && tmp.type == Fragment && tmp.key == null;
+			toChildArray(isTopLevelFragment ? tmp.props.children : tmp, newVNode._children=[], coerceToVNode, true);
 
 			if (c.getChildContext!=null) {
 				context = assign(assign({}, context), c.getChildContext());
@@ -153,7 +147,7 @@ export function diff(parentDom, newVNode, oldVNode, context, isSvg, excessDomChi
 		if (tmp = options.diffed) tmp(newVNode);
 	}
 	catch (e) {
-		catchErrorInComponent(e, newVNode._parent);
+		options._catchError(e, newVNode, oldVNode);
 	}
 
 	return newVNode._dom;
@@ -166,7 +160,7 @@ export function commitRoot(mounts, root) {
 			c.componentDidMount();
 		}
 		catch (e) {
-			catchErrorInComponent(e, c._vnode._parent);
+			options._catchError(e, c._vnode);
 		}
 	}
 
@@ -249,15 +243,15 @@ function diffElementNodes(dom, newVNode, oldVNode, context, isSvg, excessDomChil
  * Invoke or update a ref, depending on whether it is a function or object ref.
  * @param {object|function} ref
  * @param {any} value
- * @param {import('../internal').VNode} parentVNode
+ * @param {import('../internal').VNode} vnode
  */
-export function applyRef(ref, value, parentVNode) {
+export function applyRef(ref, value, vnode) {
 	try {
 		if (typeof ref=='function') ref(value);
 		else ref.current = value;
 	}
 	catch (e) {
-		catchErrorInComponent(e, parentVNode);
+		options._catchError(e, vnode);
 	}
 }
 
@@ -290,7 +284,7 @@ export function unmount(vnode, parentVNode, skipRemove) {
 				r.componentWillUnmount();
 			}
 			catch (e) {
-				catchErrorInComponent(e, parentVNode);
+				options._catchError(e, parentVNode);
 			}
 		}
 
@@ -314,16 +308,18 @@ function doRender(props, state, context) {
 /**
  * Find the closest error boundary to a thrown error and call it
  * @param {object} error The thrown value
- * @param {import('../internal').VNode} vnode The first ancestor
- * VNode check for error boundary behaviors
+ * @param {import('../internal').VNode} vnode The vnode that threw
+ * the error that was caught (except for unmounting when this parameter
+ * is the highest parent that was being unmounted)
+ * @param {import('../internal').VNode} oldVNode The oldVNode of the vnode
+ * that threw, if this VNode threw while diffing
  */
-function catchErrorInComponent(error, vnode) {
-	if (options._catchError) { options._catchError(error, vnode); }
+(options)._catchError = function (error, vnode, oldVNode) {
 
 	/** @type {import('../internal').Component} */
 	let component;
 
-	for (; vnode; vnode = vnode._parent) {
+	for (; vnode = vnode._parent;) {
 		if ((component = vnode._component) && !component._processingException) {
 			try {
 				if (component.constructor && component.constructor.getDerivedStateFromError!=null) {
@@ -344,4 +340,4 @@ function catchErrorInComponent(error, vnode) {
 	}
 
 	throw error;
-}
+};

src/internal.d.ts

@@ -12,18 +12,7 @@ export interface Options extends preact.Options {
 	/** Attach a hook that is invoked before a hook's state is queried. */
 	_hook?(component: Component): void;
 	/** Attach a hook that is invoked after an error is caught in a component but before calling lifecycle hooks */
-	_catchError?(error: any, vnode: VNode): void;
-	/**
-	 * Attach a hook that is invoked after an error is caught while executing render.
-	 *
-	 * When this hook returns true, the diffing on the affected vnode will be stopped.
-	 * When this hook returns false, the error will be thrown (and thus passed to catchError or lifecycle hooks)
-	 *
-	 * @param error The error caught
-	 * @param vnode The VNode whose component's render method threw an error
-	 * @return Return a boolean indicating whether the error was handled by the hook or not
-	 */
-	_catchRender?(error: any, newVNode: VNode, oldVNode: VNode): boolean;
+	_catchError(error: any, vnode: VNode, oldVNode: VNode | undefined): void;
 }
 
 export interface FunctionalComponent<P = {}> extends preact.FunctionComponent<P> {

test/browser/lifecycles/componentDidCatch.test.js

@@ -43,7 +43,7 @@ describe('Lifecycle methods', () => {
 				this.setState({ error });
 			}
 			render() {
-				return <div>{this.state.error ? String(this.state.error) : this.props.children}</div>;
+				return this.state.error ? String(this.state.error) : this.props.children;
 			}
 		}
 
@@ -93,6 +93,24 @@ describe('Lifecycle methods', () => {
 			expect(Receiver.prototype.componentDidCatch).to.have.been.calledWith(expectedError);
 		});
 
+		// https://github.com/preactjs/preact/issues/1570
+		it('should handle double child throws', () => {
+			const Child = ({ i }) => {
+				throw new Error(`error! ${i}`);
+			};
+
+			const fn = () => render(
+				<Receiver>
+					{[1, 2].map(i => <Child key={i} i={i} />)}
+				</Receiver>,
+				scratch
+			);
+			expect(fn).to.not.throw();
+
+			rerender();
+			expect(scratch.innerHTML).to.equal('Error: error! 2');
+		});
+
 		it('should be called when child fails in componentWillMount', () => {
 			ThrowErr.prototype.componentWillMount = throwExpectedError;
 
@@ -165,7 +183,7 @@ describe('Lifecycle methods', () => {
 					this.setState({ error });
 				}
 				render() {
-					return <div>{this.state.error ? String(this.state.error) : <ThrowErr foo={this.state.foo} />}</div>;
+					return this.state.error ? String(this.state.error) : <ThrowErr foo={this.state.foo} />;
 				}
 			}
 
@@ -192,7 +210,7 @@ describe('Lifecycle methods', () => {
 					this.setState({ error });
 				}
 				render() {
-					return <div>{this.state.error ? String(this.state.error) : <ThrowErr foo={this.state.foo} />}</div>;
+					return this.state.error ? String(this.state.error) : <ThrowErr foo={this.state.foo} />;
 				}
 			}
 
@@ -229,11 +247,7 @@ describe('Lifecycle methods', () => {
 					this.setState({ error });
 				}
 				render() {
-					return (
-						<div>
-							{this.state.error ? String(this.state.error) : <Foo ref={ref} />}
-						</div>
-					);
+					return this.state.error ? String(this.state.error) : <Foo ref={ref} />;
 				}
 			}
 
@@ -256,11 +270,7 @@ describe('Lifecycle methods', () => {
 					this.setState({ error });
 				}
 				render() {
-					return (
-						<div>
-							{this.state.error ? String(this.state.error) : <div ref={ref} />}
-						</div>
-					);
+					return this.state.error ? String(this.state.error) : <div ref={ref} />;
 				}
 			}