preactjs / signals

Manage state with style in every framework
https://preactjs.com/blog/introducing-signals/
MIT License
3.64k stars 89 forks source link

Signal not re-rendering components when extending PureComponent due to shouldComponentUpdate #432

Closed harrypunia closed 8 months ago

harrypunia commented 8 months ago

I am having trouble setting up signals to re-render a class component that extends PureComponent (preact/compat). The signals are able to re-render the component when, either not returning any value on shouldComponentUpdate or using Component instead of PureComponent.

Currently using preact v10.17.1 and @preact/signals v1.2.1.

rschristian commented 8 months ago

Can you please provide a reproduction? Helps make sure we're looking at the same thing.

harrypunia commented 8 months ago

Sure!

Steps to reproduce:

  1. Create a signal inside a global js file
  2. Import the signal and display inside a component that extends React.PureComponent
  3. Create a trigger component that changes the value of the signal
  4. Render these components beside each other to test the reactive behavior

The problem

Signal only re-renders display component if either:

The display component uses React.Component (which does not support shouldComponentUpdate) instead of PureComponent

OR

Overwrites shouldComponentUpdate and not return anything

Code would look something like this:

signal.js

import {signal} from "@preact/signals";

export default {
  number: signal(0)
}

Display component

import React from "react";
import signal from "signal.js";

class DisplayComponent extends React.PureComponent {

    shouldComponentUpdate(nextProps, nextState, nextContext) {
      //returning nothing allows signal to re-render this component. removing this method or return super.shouldComponentUpdate(nextProps, nextState, nextContext) will re-create the problem
    }

    render() {
      const value = signal.number.value
      return (
        <p>{value}</p>
      )
    }

}

Trigger Component

import React from "react";
import signal from "signal.js"

class TriggerComponent extends React.PureComponent {

    render() {
      return (
        <button onClick={() => signal.number.value++}>Increase Value</button>
      )
    }

}

My assumption is that shouldComponentUpdate is preventing the component to update? Signal values are changing a-okay. Also when adding logs to the shouldComponentUpdate it does log every time the signal value updates.

rschristian commented 8 months ago

Hm, and you're expecting the component to update regardless of the fact that you're using PureComponent/a sCU that says it shouldn't?

I'd say this is operating as intended, tbh. PureComponent is a component with a sCU limiting rerenders to changes in props & state, of which the signal is neither. Using PureComponent is telling Preact not to rerender (unless you override the implementation, which makes little sense btw; just use Component if you're providing your own sCU implementation).

harrypunia commented 8 months ago

mmmm, Unfortunately for us we have a PureComponent super class extended by almost all components on the project and sCU does bring value in a situation like this. Do you think there any way compare signal value during sCU, perhaps using signal inside a state and causing a setState instead of forceUpdate (assuming thats what signal change triggers)? or am I just limited to Component?

rschristian commented 8 months ago

and sCU does bring value in a situation like this.

What's the value? The intended way of using signals should solve most (if not all) unnecessary rerenders.

harrypunia commented 8 months ago

Mainly just finer control on props re-render, but I see your point. Thanks for the help!