Signal doesn't update for style/class attributes in a Show component

[ovenbreado]

• Check if updating to the latest version resolves the issue

Environment

• I am using @preact/signals-core
• I am using @preact/signals
• I am using @preact/signals-react
  • I understand usage changed in v2, and I've followed the React Integration instructions

Describe the bug

Edit:
Check the comment I have below in #882 (comment) for a more minimal repro. Essentially the block caused signals within attributes to be unsubscribed (a bug within createPropUpdater from what I understand).

This is fixed by @JoviDeCroock via #883 , massive props to him!

---

New user of preact here! Used to solid.js but wanted something lightweight.
I've noticed that when using Show to show/hide an element that it doesn't reflect in the div attributes properly (I do ack that attributes are currently experimental). Am I doing something conceptually wrong or is this a bug? thanks!
You can reproduce this in the tutorial sandbox.

To Reproduce

Steps to reproduce the behavior:

Copy the following code snippet into https://preactjs.com/tutorial/01-vdom, then press any key a few times. The colors should follow the text on it; R for red, B for blue, G for green, but it doesn't. The s following some text is a switch to switch to the second Show when it is in the string.

The following code has a switch to two different sections when the array is rotated.

import { useRef, useEffect, useState, useCallback } from 'preact/hooks';
import { useSignal, useSignalEffect, useComputed, batch, computed, effect, signal } from '@preact/signals';
import { For, Show } from '@preact/signals/utils';

const COLOR_CLASSES = {
  "R": 'background:#f00',
  "B": 'background:#00f',
  "G": 'background:#0f0',
};

export function App(props) {
  const sigArray = useSignal([
    useSignal('R'),
    useSignal('Bs'),
    useSignal('G'),
    useSignal('Rs')
  ]); // Array of 4 cubes, s is a special mark, renders a smaller div

  useEffect(() => {
    window.addEventListener('keydown', (e) => {
      let keep = sigArray.value[0].value;
      sigArray.value[0].value = sigArray.value[1].value
      sigArray.value[1].value = sigArray.value[2].value
      sigArray.value[2].value = sigArray.value[3].value
      sigArray.value[3].value = keep
    });
  }, []);
  
  return (
    <div class="">
      <For each={sigArray}>
        {(col, colIndex) => {
          return (
            <div>
              <Show when={useComputed(() => col.value?.charAt(1) !== "s")}>
                <div style={useComputed(() => `width:30px;height:30px; ${COLOR_CLASSES[col.value?.charAt(0)]}`)}>
                  {col}
                </div>
              </Show>
              <Show when={useComputed(() => col.value?.charAt(1) === "s")}>
                <div style={useComputed(() => `width:20px;height:20px; ${COLOR_CLASSES[col.value?.charAt(0)]}`)}>
                  {col}
                </div>
              </Show>
            </div>
          );
        }}
      </For>
    </div>
  );
}

render(<App />, document.getElementById('app'));

Expected behavior
Signal should update background colors attribute according to the letters.

Thanks for taking a look into this!

[JoviDeCroock]

Does solid allow you to mutate signals, I don't think so 😅 you should immutably update your array and it will work. Also leveraging hooks inside of a loop is highly discouraged because this will skew the amount of hooks you render each time so you should hoist the return value of your For loop into a new component to make the amount of state you create deterministic rather than dependent on your array-ordering and length.

[ovenbreado]

Woah that was quick!

I was trying to emulate something similar to Solidjs's store(rather than signal) where, iirc, it updates only properties that change. I may have gone a little far out trying to do that, but I'll try the hoisting method that you've suggested in a bit.

[JoviDeCroock]

Well, that is the case if you just return a signal in an attribute/JSX-node it will only update that node. We also have our models #876 which are similar to stores.

[ovenbreado]

I did

sigArray.value = [sigArray.value[1], sigArray.value[2], sigArray.value[3], sigArray.value[0]]

to mutably update the array, but the 4 components in the keeps rerendering. Is that intended? Maybe I had the idea that it shouldn't do that - e.g. reuse the dom parts in that array.

Otherwise, it works doing the above.

Thank you so much btw! - this issue can be closed.

[JoviDeCroock]

Signals in Preact are the middle ground, we have optimizations like attributes/text-nodes but generally logic involving components cause re-renders. I have a lot of blog posts you can read up on if you like (signals tag) https://jovidecroock.com/blog

[ovenbreado]

I see. I'll defeinitely take a quick read through the blog. Thanks again!

[ovenbreado]

Sorry for pinging this. I have a simpler example of what I'm talking about:

pressing any key cycles [red, blue, green] once then [red, green, green] forever. I expected it to cycle [red, blue, green].

Is this a limitation of signals in attributes on dom nodes that are unmounted or a bug?

thanks!

Copy the following code snippet into https://preactjs.com/tutorial/01-vdom :

import { useRef, useEffect, useState, useCallback } from 'preact/hooks';
import { useSignal, useSignalEffect, useComputed, batch, computed, effect, signal } from '@preact/signals';
import { For, Show } from '@preact/signals/utils';

const colorList = [
  'background:#f00',
  'background:#00f',
  'background:#0f0',
];

export function App(props) {
  const counter = useSignal(0);
  const isZero = useComputed(() => counter.value == 0);
  const isNotZero = useComputed(() => counter.value != 0);
  const isZeroClass = useComputed(() => `width:30px;height:30px; ${colorList[counter.value]}`);
  const isNotZeroClass = useComputed(() => `width:20px;height:20px; ${colorList[counter.value]}`);
  
  useEffect(() => {
    window.addEventListener('keydown', () => counter.value = (counter.value + 1) % 3);
  }, []);
  
  return (
    <div>
      <Show when={isZero}>
        <div style={isZeroClass}>
          {counter}
        </div>
      </Show>
      <Show when={isNotZero}>
        <div style={isNotZeroClass}>
          {counter}
        </div>
      </Show>
    </div>
  );
}

render(<App />, document.getElementById('app'));

[JoviDeCroock]

Fixed in #883

I think the direct updaters are a bit underused atm which can lead to finding these footguns

[ovenbreado]

Amazing stuff! I'll update the first comment to reflect this is someone happens onto this issue.

I'll also patch it in and see if its working for my use case.

Much thanks :)

---

Edit: It works. Tested this by patching it in using bun.

[JoviDeCroock]

It's also released in the newest preact-signals patch version