RFC: Remove HAS_HOOKS_STATE bit

[JoviDeCroock]

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]

⚠️ No Changeset found

Latest commit: 548142a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[netlify]

✅ Deploy Preview for preact-signals-demo ready!

Name	Link
🔨 Latest commit	548142a
🔍 Latest deploy log	https://app.netlify.com/sites/preact-signals-demo/deploys/67652172dbf9b8000853d0ad
😎 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: -69 B (-0.08%)

Total Size: 83.2 kB

Filename	Size	Change
docs/dist/assets/bench.********.js	1.59 kB	+4 B (+0.25%)
docs/dist/assets/index.********.js	837 B	+1 B (+0.12%)
docs/dist/assets/signals.module.********.js	2.14 kB	-21 B (-0.97%)
docs/dist/basic-********.js	244 B	+1 B (+0.41%)
docs/dist/demos-********.js	3.45 kB	+3 B (+0.09%)
packages/preact/dist/signals.js	1.43 kB	-32 B (-2.19%)
packages/preact/dist/signals.mjs	1.41 kB	-25 B (-1.74%)

ℹ️ View Unchanged

Filename	Size
docs/dist/assets/client.********.js	46.4 kB
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}