Merge branch 'master' into feat/rework-debug-tests

compat/mangle.json

@@ -20,6 +20,7 @@
       "$_root": "__p",
       "$_commit": "__c",
       "$_suspensions": "__u",
+      "$_fallback": "__f",
       "$_diff": "__b",
       "$_render": "__r",
       "$_hook": "__h",

hooks/mangle.json

@@ -6,6 +6,7 @@
   "props": {
     "cname": 6,
     "props": {
+      "$_afterPaintQueued": "__a",
       "$__hooks": "__H",
       "$_component": "__c",
       "$_defaultValue": "__p",

hooks/src/index.js

@@ -9,6 +9,9 @@ let currentComponent;
 /** @type {Array<import('./internal').Component>} */
 let afterPaintEffects = [];
 
+/** @type {Array<import('./internal').Component>} */
+let layoutEffects = [];
+
 let oldBeforeRender = options._render;
 options._render = vnode => {
 	if (oldBeforeRender) oldBeforeRender(vnode);
@@ -17,7 +20,9 @@ options._render = vnode => {
 	currentIndex = 0;
 
 	if (currentComponent.__hooks) {
-		currentComponent.__hooks._pendingEffects = handleEffects(currentComponent.__hooks._pendingEffects);
+		currentComponent.__hooks._pendingEffects.forEach(invokeCleanup);
+		currentComponent.__hooks._pendingEffects.forEach(invokeEffect);
+		currentComponent.__hooks._pendingEffects = [];
 	}
 };
 
@@ -30,10 +35,22 @@ options.diffed = vnode => {
 
 	const hooks = c.__hooks;
 	if (hooks) {
-		hooks._pendingLayoutEffects = handleEffects(hooks._pendingLayoutEffects);
+		if (hooks._pendingLayoutEffects.length) {
+			layoutEffects.push(c);
+		}
+
+		if (hooks._pendingEffects.length) {
+			afterPaint(afterPaintEffects.push(c));
+		}
 	}
 };
 
+let oldCommit = options._commit;
+options._commit = vnode => {
+	if (oldCommit) oldCommit(vnode);
+	flushLayoutEffects();
+};
+
 
 let oldBeforeUnmount = options.unmount;
 options.unmount = vnode => {
@@ -117,7 +134,6 @@ export function useEffect(callback, args) {
 		state._args = args;
 
 		currentComponent.__hooks._pendingEffects.push(state);
-		afterPaint(currentComponent);
 	}
 }
 
@@ -132,6 +148,7 @@ export function useLayoutEffect(callback, args) {
 	if (argsChanged(state._args, args)) {
 		state._value = callback;
 		state._args = args;
+
 		currentComponent.__hooks._pendingLayoutEffects.push(state);
 	}
 }
@@ -206,20 +223,33 @@ export function useDebugValue(value, formatter) {
 // then effects will ALWAYS run on the NEXT frame instead of the current one, incurring a ~16ms delay.
 // Perhaps this is not such a big deal.
 /**
- * Invoke a component's pending effects after the next frame renders
- * @type {(component: import('./internal').Component) => void}
+ * Schedule afterPaintEffects flush after the browser paints
+ * @type {(newQueueLength: number) => void}
  */
 /* istanbul ignore next */
 let afterPaint = () => {};
 
+/**
+ * Layout effects consumer
+ */
+function flushLayoutEffects() {
+	layoutEffects.some(component => {
+		component.__hooks._pendingLayoutEffects.forEach(invokeCleanup);
+		component.__hooks._pendingLayoutEffects.forEach(invokeEffect);
+		component.__hooks._pendingLayoutEffects = [];
+	});
+	layoutEffects = [];
+}
+
 /**
  * After paint effects consumer.
  */
 function flushAfterPaintEffects() {
 	afterPaintEffects.some(component => {
-		component._afterPaintQueued = 0;
 		if (component._parentDom) {
-			component.__hooks._pendingEffects = handleEffects(component.__hooks._pendingEffects);
+			component.__hooks._pendingEffects.forEach(invokeCleanup);
+			component.__hooks._pendingEffects.forEach(invokeEffect);
+			component.__hooks._pendingEffects = [];
 		}
 	});
 	afterPaintEffects = [];
@@ -248,11 +278,8 @@ function afterNextFrame(callback) {
 /* istanbul ignore else */
 if (typeof window !== 'undefined') {
 	let prevRaf = options.requestAnimationFrame;
-	afterPaint = (component) => {
-		if (
-			(!component._afterPaintQueued && (component._afterPaintQueued = afterPaintEffects.push(component)) === 1)
-			|| prevRaf !== options.requestAnimationFrame
-		) {
+	afterPaint = (newQueueLength) => {
+		if (newQueueLength === 1 || prevRaf !== options.requestAnimationFrame) {
 			prevRaf = options.requestAnimationFrame;
 
 			/* istanbul ignore next */
@@ -261,15 +288,6 @@ if (typeof window !== 'undefined') {
 	};
 }
 
-/**
- * @param {import('./internal').EffectHookState[]} effects
- */
-function handleEffects(effects) {
-	effects.forEach(invokeCleanup);
-	effects.forEach(invokeEffect);
-	return [];
-}
-
 /**
  * @param {import('./internal').EffectHookState} hook
  */

hooks/test/browser/combinations.test.js

@@ -214,4 +214,68 @@ describe('combinations', () => {
 
 		expect(scratch.textContent).to.equal('01');
 	});
+
+	it('should have a right call order with correct dom ref', () => {
+		let i = 0, set;
+		const calls = [];
+
+		function Inner() {
+			useLayoutEffect(() => {
+				calls.push('layout inner call ' + scratch.innerHTML);
+				return () => calls.push('layout inner dispose ' + scratch.innerHTML);
+			});
+			useEffect(() => {
+				calls.push('effect inner call ' + scratch.innerHTML);
+				return () => calls.push('effect inner dispose ' + scratch.innerHTML);
+			});
+			return <span>hello {i}</span>;
+		}
+
+		function Outer() {
+			i++;
+			const [state, setState] = useState(false);
+			set = () => setState(!state);
+			useLayoutEffect(() => {
+				calls.push('layout outer call ' + scratch.innerHTML);
+				return () => calls.push('layout outer dispose ' + scratch.innerHTML);
+			});
+			useEffect(() => {
+				calls.push('effect outer call ' + scratch.innerHTML);
+				return () => calls.push('effect outer dispose ' + scratch.innerHTML);
+			});
+			return <Inner />;
+		}
+
+		act(() => render(<Outer />, scratch));
+		expect(calls).to.deep.equal([
+			'layout inner call <span>hello 1</span>',
+			'layout outer call <span>hello 1</span>',
+			'effect inner call <span>hello 1</span>',
+			'effect outer call <span>hello 1</span>'
+		]);
+
+		// NOTE: this order is (at the time of writing) intentionally different from
+		// React. React calls all disposes across all components, and then invokes all
+		// effects across all components. We call disposes and effects in order of components:
+		// for each component, call its disposes and then its effects. If presented with a
+		// compelling use case to support inter-component dispose dependencies, then rewrite this
+		// test to test React's order. In other words, if there is a use case to support calling
+		// all disposes across components then re-order the lines below to demonstrate the desired behavior.
+
+		act(() => set());
+		expect(calls).to.deep.equal([
+			'layout inner call <span>hello 1</span>',
+			'layout outer call <span>hello 1</span>',
+			'effect inner call <span>hello 1</span>',
+			'effect outer call <span>hello 1</span>',
+			'layout inner dispose <span>hello 2</span>',
+			'layout inner call <span>hello 2</span>',
+			'layout outer dispose <span>hello 2</span>',
+			'layout outer call <span>hello 2</span>',
+			'effect inner dispose <span>hello 2</span>',
+			'effect inner call <span>hello 2</span>',
+			'effect outer dispose <span>hello 2</span>',
+			'effect outer call <span>hello 2</span>'
+		]);
+	});
 });

hooks/test/browser/useContext.test.js

@@ -242,4 +242,49 @@ describe('useContext', () => {
 			done();
 		});
 	});
+
+	it('should not rerender consumers that have been unmounted', () => {
+		const context = createContext(0);
+		const Provider = context.Provider;
+
+		const Inner = sinon.spy(() => {
+			const value = useContext(context);
+			return <div>{value}</div>;
+		});
+
+		let toggleConsumer;
+		let changeValue;
+		class App extends Component {
+			constructor() {
+				super();
+
+				this.state = { value: 0, show: true };
+				changeValue = value => this.setState({ value });
+				toggleConsumer = () => this.setState(({ show }) => ({ show: !show }));
+			}
+			render(props, state) {
+				return (
+					<Provider value={state.value}>
+						<div>{state.show ? <Inner /> : null}</div>
+					</Provider>
+				);
+			}
+		}
+
+		render(<App />, scratch);
+		expect(scratch.innerHTML).to.equal('<div><div>0</div></div>');
+		expect(Inner).to.have.been.calledOnce;
+
+		act(() => changeValue(1));
+		expect(scratch.innerHTML).to.equal('<div><div>1</div></div>');
+		expect(Inner).to.have.been.calledTwice;
+
+		act(() => toggleConsumer());
+		expect(scratch.innerHTML).to.equal('<div></div>');
+		expect(Inner).to.have.been.calledTwice;
+
+		act(() => changeValue(2));
+		expect(scratch.innerHTML).to.equal('<div></div>');
+		expect(Inner).to.have.been.calledTwice;
+	});
 });

hooks/test/browser/useEffect.test.js

@@ -63,7 +63,7 @@ describe('useEffect', () => {
 		});
 	});
 
-	it('Should execute effects in the right order', () => {
+	it('should execute multiple effects in same component in the right order', () => {
 		let executionOrder = [];
 		const App = ({ i }) => {
 			executionOrder = [];