Merge pull request #3553 from preactjs/state-settling-x

implement state settling in X

hooks/src/index.js

@@ -6,6 +6,9 @@ let currentIndex;
 /** @type {import('./internal').Component} */
 let currentComponent;
 
+/** @type {import('./internal').Component} */
+let previousComponent;
+
 /** @type {number} */
 let currentHook = 0;
 
@@ -34,10 +37,19 @@ options._render = vnode => {
 
  const hooks = currentComponent.__hooks;
  if (hooks) {
- hooks._pendingEffects.forEach(invokeCleanup);
- hooks._pendingEffects.forEach(invokeEffect);
- hooks._pendingEffects = [];
+ if (previousComponent === currentComponent) {
+ hooks._pendingEffects = [];
+ currentComponent._renderCallbacks = [];
+ hooks._list.forEach(hookItem => {
+ if (hookItem._args) hookItem._args = undefined;
+ });
+ } else {
+ hooks._pendingEffects.forEach(invokeCleanup);
+ hooks._pendingEffects.forEach(invokeEffect);
+ hooks._pendingEffects = [];
+ }
  }
+ previousComponent = currentComponent;
 };
 
 options.diffed = vnode => {
@@ -48,6 +60,7 @@ options.diffed = vnode => {
  afterPaint(afterPaintEffects.push(c));
  }
  currentComponent = null;
+ previousComponent = null;
 };
 
 options._commit = (vnode, commitQueue) => {
@@ -202,7 +215,7 @@ export function useImperativeHandle(ref, createHandle, args) {
  return () => ref(null);
  } else if (ref) {
  ref.current = createHandle();
- return () => ref.current = null;
+ return () => (ref.current = null);
  }
  },
  args == null ? args : args.concat(ref)

hooks/test/browser/useEffect.test.js

@@ -503,4 +503,30 @@ describe('useEffect', () => {
  'Parent - Effect'
  ]);
  });
+
+ it('should cancel effects from a disposed render', () => {
+ const calls = [];
+ const App = () => {
+ const [greeting, setGreeting] = useState('bye');
+
+ useEffect(() => {
+ calls.push('doing effect' + greeting);
+ return () => {
+ calls.push('cleaning up' + greeting);
+ };
+ }, [greeting]);
+
+ if (greeting === 'bye') {
+ setGreeting('hi');
+ }
+
+ return <p>{greeting}</p>;
+ };
+
+ act(() => {
+ render(<App />, scratch);
+ });
+ expect(calls.length).to.equal(1);
+ expect(calls).to.deep.equal(['doing effecthi']);
+ });
 });

hooks/test/browser/useLayoutEffect.test.js

@@ -386,4 +386,30 @@ describe('useLayoutEffect', () => {
  'Parent - Effect'
  ]);
  });
+
+ it('should cancel effects from a disposed render', () => {
+ const calls = [];
+ const App = () => {
+ const [greeting, setGreeting] = useState('bye');
+
+ useLayoutEffect(() => {
+ calls.push('doing effect' + greeting);
+ return () => {
+ calls.push('cleaning up' + greeting);
+ };
+ }, [greeting]);
+
+ if (greeting === 'bye') {
+ setGreeting('hi');
+ }
+
+ return <p>{greeting}</p>;
+ };
+
+ act(() => {
+ render(<App />, scratch);
+ });
+ expect(calls.length).to.equal(1);
+ expect(calls).to.deep.equal(['doing effecthi']);
+ });
 });

hooks/test/browser/useState.test.js

@@ -1,4 +1,4 @@
-import { setupRerender } from 'preact/test-utils';
+import { setupRerender, act } from 'preact/test-utils';
 import { createElement, render } from 'preact';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
 import { useState } from 'preact/hooks';
@@ -211,4 +211,26 @@ describe('useState', () => {
  rerender();
  expect(scratch.innerHTML).to.equal('');
  });
+
+ it('should render a second time when the render function updates state', () => {
+ const calls = [];
+ const App = () => {
+ const [greeting, setGreeting] = useState('bye');
+
+ if (greeting === 'bye') {
+ setGreeting('hi');
+ }
+
+ calls.push(greeting);
+
+ return <p>{greeting}</p>;
+ };
+
+ act(() => {
+ render(<App />, scratch);
+ });
+ expect(calls.length).to.equal(2);
+ expect(calls).to.deep.equal(['bye', 'hi']);
+ expect(scratch.textContent).to.equal('hi');
+ });
 });

src/diff/index.js

@@ -170,15 +170,29 @@ export function diff(
 
  c.context = componentContext;
  c.props = newProps;
- c.state = c._nextState;
-
- if ((tmp = options._render)) tmp(newVNode);
-
- c._dirty = false;
  c._vnode = newVNode;
  c._parentDom = parentDom;
 
- tmp = c.render(c.props, c.state, c.context);
+ let renderHook = options._render,
+ count = 0;
+ if ('prototype' in newType && newType.prototype.render) {
+ c.state = c._nextState;
+ c._dirty = false;
+
+ if (renderHook) renderHook(newVNode);
+
+ tmp = c.render(c.props, c.state, c.context);
+ } else {
+ do {
+ c._dirty = false;
+ if (renderHook) renderHook(newVNode);
+
+ tmp = c.render(c.props, c.state, c.context);
+
+ // Handle setState called in render, see #2553
+ c.state = c._nextState;
+ } while (c._dirty && ++count < 25);
+ }
 
  // Handle setState called in render, see #2553
  c.state = c._nextState;