Merge 01abe2a into 0246cd8

hooks/src/index.js

@@ -17,8 +17,7 @@ options.render = vnode => {
 	currentIndex = 0;
 
 	if (!currentComponent.__hooks) return;
-	currentComponent.__hooks._pendingEffects.forEach(invokeEffect);
-	currentComponent.__hooks._pendingEffects = [];
+	currentComponent.__hooks._pendingEffects = handleEffects(currentComponent.__hooks._pendingEffects);
 };
 
 
@@ -34,8 +33,7 @@ options.diffed = vnode => {
 
 	// TODO: Consider moving to a global queue. May need to move
 	// this to the `commit` option
-	hooks._pendingLayoutEffects.forEach(invokeEffect);
-	hooks._pendingLayoutEffects = [];
+	hooks._pendingLayoutEffects = handleEffects(hooks._pendingLayoutEffects);
 };
 
 
@@ -196,8 +194,7 @@ function flushAfterPaintEffects() {
 	afterPaintEffects.forEach(component => {
 		component._afterPaintQueued = false;
 		if (!component._parentDom) return;
-		component.__hooks._pendingEffects.forEach(invokeEffect);
-		component.__hooks._pendingEffects = [];
+		component.__hooks._pendingEffects = handleEffects(component.__hooks._pendingEffects);
 	});
 	afterPaintEffects = [];
 }
@@ -220,12 +217,21 @@ if (typeof window !== 'undefined') {
 	};
 }
 
+function handleEffects(effects) {
+	effects.forEach(invokeCleanup);
+	effects.forEach(invokeEffect);
+	return [];
+}
+
+function invokeCleanup(hook) {
+	if (hook._cleanup) hook._cleanup();
+}
+
 /**
  * Invoke a Hook's effect
  * @param {import('./internal').EffectHookState} hook
  */
 function invokeEffect(hook) {
-	if (hook._cleanup) hook._cleanup();
 	const result = hook._value();
 	if (typeof result === 'function') hook._cleanup = result;
 }

hooks/test/browser/useEffect.test.js

@@ -1,4 +1,4 @@
-import { setupRerender } from 'preact/test-utils';
+import { act } from 'preact/test-utils';
 import { createElement as h, render } from 'preact';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
 import { useEffect } from '../../src';
@@ -62,4 +62,23 @@ describe('useEffect', () => {
 			expect(callback).to.not.be.called;
 		});
 	});
+
+	it('Should execute effects in the right order', () => {
+		let executionOrder = [];
+		const App = ({ i }) => {
+			executionOrder = [];
+			useEffect(() => {
+				executionOrder.push('action1');
+				return () => executionOrder.push('cleanup1');
+			}, [i]);
+			useEffect(() => {
+				executionOrder.push('action2');
+				return () => executionOrder.push('cleanup2');
+			}, [i]);
+			return <p>Test</p>;
+		};
+		act(() => render(<App i={0} />, scratch));
+		act(() => render(<App i={2} />, scratch));
+		expect(executionOrder).to.deep.equal(['cleanup1', 'cleanup2', 'action1', 'action2']);
+	});
 });

hooks/test/browser/useLayoutEffect.test.js

@@ -1,4 +1,4 @@
-import { setupRerender } from 'preact/test-utils';
+import { act } from 'preact/test-utils';
 import { createElement as h, render } from 'preact';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
 import { useEffectAssertions } from './useEffectAssertions.test';
@@ -69,4 +69,23 @@ describe('useLayoutEffect', () => {
 
 		expect(callback).to.be.calledOnce;
 	});
+
+	it('Should execute layout effects in the right order', () => {
+		let executionOrder = [];
+		const App = ({ i }) => {
+			executionOrder = [];
+			useLayoutEffect(() => {
+				executionOrder.push('action1');
+				return () => executionOrder.push('cleanup1');
+			}, [i]);
+			useLayoutEffect(() => {
+				executionOrder.push('action2');
+				return () => executionOrder.push('cleanup2');
+			}, [i]);
+			return <p>Test</p>;
+		};
+		act(() => render(<App i={0} />, scratch));
+		act(() => render(<App i={2} />, scratch));
+		expect(executionOrder).to.deep.equal(['cleanup1', 'cleanup2', 'action1', 'action2']);
+	});
 });