RFC: Remove HAS_HOOKS_STATE bit

[JoviDeCroock]

Resolves preactjs/preact#4628

This is a thought exercise as a result of a fix in the Preact respository, strap in as this might become a doozy, open up a Stackblitz and follow along. In 10.25.0 we fixed an issue where the hooks-state settling logic would prevent a component having both signals as well as state from re-rendering, The relevant PR.

The issue was that when a component has a signal that updates and hooks that end up at the same value the component would not rerender.

import { useEffect, useState } from 'preact/hooks';
import { useSignal } from '@preact/signals';

// In 10.25.x this will correctly rerender.
// In 10.12.x-10.24.x and before this won't rerender.
let rootRenders = 0;
export function App() {
  const [count, setCount] = useState(0);
  const signal = useSignal('hello world');

  useEffect(() => {
    setCount(1);
    setCount(0);
    signal.value = 'bye';
  }, []);

  return (
    <div>
      <p>Count: {count} - App Rendered {rootRenders++} times</p>
      <p>{signal.value}</p>
      <Signal />
    </div>
  );
}

let renders = 0;
function Signal() {
  const signal = useSignal('hello world');
  return (
    <p>
      {signal} - Signal Rendered {renders++} times
    </p>
  );
}

This was solved by calling the previous shouldComponentUpdate (in this case the global sCU set by @preact/signals) when the state settling logic decides to bail out. From a plugin perspective this is the right course of action as we want all plugins to have their lifecycles respected.

Now this does however bring us to the... interesting part, when a state hook now decides in 10.25.x that it needs to bail we'll rerender it anyway because we see HAS_HOOK_STATE.

Let's look at this in action

import { useEffect, useState } from 'preact/hooks';
import { useSignal } from '@preact/signals';
import './app.css';

// In 10.25.x the App will rerender despite settling on the same value
// In 10.12.x-10.24.x and before this won't rerender as our state settles.
let rootRenders = 0;
export function App() {
  const [count, setCount] = useState(0);

  useEffect(() => {
    setCount(1);
    setCount(0);
  }, []);

  return (
    <div>
      <p>
        Count: {count} - App Rendered {rootRenders++} times
      </p>
      <p>{signal.value}</p>
      <Signal />
    </div>
  );
}

let renders = 0;
function Signal() {
  const signal = useSignal('hello world');
  return (
    <p>
      {signal} - Signal Rendered {renders++} times
    </p>
  );
}

Now what I want to achieve is a middle ground where we can respect hooks state settling as well as rerender correctly.

Another variable to consider with signals is hookState._component.props !== props;, this line is meant to respect the top-down rerender flow. When the parent initiates a render and passes through an updated prop we want to rerender deeply as this is the classic state based Virtual DOM way. In signals we do shallow equality, this however means that any component having both signals and state-based VDOM will respect the top-down render which I guess is a "one up" for removing this HAS_HOOKS_STATE.

Now the question becomes how will this behave?

10.0-10.12

• State settles to the same value --> we rely on HAS_HOOKS_STATE to rerender the component
  • This would break :(
• Signal is updated in a hooks component --> we rely on HAS_PENDING_UPDATE to rerender the component
  • This would keep working :)

10.12-10.24

I think it's safe to not cater to this case as it was a bug in Preact.

• State settles to the same value --> signal sCU gets called and triggers a rerender with HAS_HOOKS_STATE
• Signal is updated in a hooks component
  • The underlying sCU is called if the state has no pending updates

10.25.x and onwards

• State settles to the same value --> signal sCU would not get called
  • Can ignore any state update coming from signals when the state is changed but settles on the same value
• Signal is updated in a hooks component --> we rely on HAS_PENDING_UPDATE to rerender the component

I don't want to imply that removing HAS_HOOKS_STATE is the right solution but this is more a way to discuss the tradeoffs and show the impact it has on different versions of Preact.

Note

The consequences of the above also become clear when upgrading the Preact package in signals, the currently failing test passes (with the current changes) when we upgrade to anything beyond 10.12.x. This implies that this bit is only needed for 10.0-10.12

[changeset-bot]

🦋 Changeset detected

Latest commit: 17ea2e4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@preact/signals	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for preact-signals-demo ready!

Name	Link
🔨 Latest commit	17ea2e4
🔍 Latest deploy log	https://app.netlify.com/sites/preact-signals-demo/deploys/67811c46f9122a00082de032
😎 Deploy Preview	https://deploy-preview-630--preact-signals-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[github-actions]

Size Change: +124 B (+0.15%)

Total Size: 83.2 kB

Filename	Size	Change
docs/dist/assets/bench.********.js	1.59 kB	+2 B (+0.13%)
docs/dist/assets/signals.module.********.js	2.2 kB	+39 B (+1.81%)
docs/dist/basic-********.js	244 B	+1 B (+0.41%)
docs/dist/demos-********.js	3.45 kB	+2 B (+0.06%)
packages/preact/dist/signals.js	1.5 kB	+36 B (+2.46%)
packages/preact/dist/signals.mjs	1.48 kB	+44 B (+3.07%)

ℹ️ View Unchanged

Filename	Size
docs/dist/assets/client.********.js	46.2 kB
docs/dist/assets/index.********.js	836 B
docs/dist/assets/jsxRuntime.module.********.js	284 B
docs/dist/assets/preact.module.********.js	4.03 kB
docs/dist/assets/signals-core.module.********.js	1.41 kB
docs/dist/assets/style.********.js	21 B
docs/dist/assets/style.********.css	1.24 kB
docs/dist/nesting-********.js	1.13 kB
docs/dist/react-********.js	242 B
packages/core/dist/signals-core.js	1.45 kB
packages/core/dist/signals-core.mjs	1.47 kB
packages/react-transform/dist/signals-*********.js	4.93 kB
packages/react-transform/dist/signals-transform.mjs	4.16 kB
packages/react-transform/dist/signals-transform.umd.js	5.04 kB
packages/react/dist/signals.js	188 B
packages/react/dist/signals.mjs	150 B

_{compressed-size-action}

[JoviDeCroock]

Update, we should remove the base rerender when there are no signals to also account for hooks-state as seen in 2c97a21 else state settling in hooks would never be respected.

I've added a heuristic for whether or not we are dealing with state settling, you can see the relevant property here - I would backport this to 1.3.x as well to have safety in both major versions

[JoviDeCroock]

I'd love to have opinions on this retroactive fix, with adding this.u here we don't necessarily do anything wrong, when hooks would settle into a falsey update they would never call this function before so as of 10.25.x it fixes an issue and in pre 10.24.x it doesn't really do anything