preactjs / signals

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

Signals value do not trigger the component rerender if used in the return statement in JSX #478

Closed GioOli7 closed 9 months ago

GioOli7 commented 9 months ago

Hello everyone, I'm new to Signals, and while running some tests, I'm noticing a rather strange issue. I'm experiencing an issue with using Signal values within the return statement in JSX. I've noticed that if I directly use Signal values in the return statement, the component doesn't re-render, and thus, my UI doesn't react to changes in the state of variables initialized with Signals. To make my application work correctly, I have to extract the logic and components to be rendered into a variable outside the return statement and only reference the variable in the return statement.

I show some code as an example.

userData from useGoogleAuth() is Signals variable stored in a separate file

In this example my component do not react to the signals change

image

to make it work, i need to extract the logic outside like this:

image

I'm using @preact/signals-react and Vite

It's a bug or i'm doing it wrong?

Thanks in advance for any help.

XantreDev commented 9 months ago

Are you using the babel plugin? Send vite config please

GioOli7 commented 9 months ago

No, this is my vite config

import { defineConfig } from 'vite'
import react from '@vitejs/plugin-react'

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [react()],
})
rschristian commented 9 months ago

Don't provide reproductions as screenshots please.

If you're not using the Babel plugin, then you need to be calling useSignals(). Please have a read through of the instructions.

Edit: Also, you should not ever call computed in a component. This is not stable. Use useComputed, again, per the docs.

GioOli7 commented 9 months ago

Thanks for help, I was using this documentation Now I using babel plugin and the component is react again. But i don't get it. Now, however, a change in the value of the Signals variable triggers me to render all sibling and child components as if I were using useState.

This is a code example. with the implementation shown in the second screenshot I could open and close the modal without triggering the rendering of the main component. Now instead both the main component and the Test component are re-rendered. In its current state, it is as if I were using useState


export const isExpenseModalOpen = signal(false);
import Test from '../generic/Test';

function Main() {
  console.log("main rerender")
  const {getAllData, updateData} = useFirebase()
  const [allData, setAllData] = useState({})

  useEffect(()=>{
        async function fetch(){
            setAllData(await getAllData())
        }
        // fetch()
  },[])

  // const addExpenseModal = computed(() => {
  //   if(isExpenseModalOpen.value === true){
  //     return (
  //       <Modal title="add" >
  //           <div>ciao</div>
  //       </Modal> 
  //     )
  //   }
  // });

  return (
    <div className='main'>
      <button className='addExpense' onClick={() => isExpenseModalOpen.value = !isExpenseModalOpen.value}>
        +
      </button>
      { 
        isExpenseModalOpen.value && (
          <Modal title="add" >
              <div>ciao</div>
          </Modal> 
        )
      }
      <Test />

    </div>
  )
}

This is the logs when open and close the modal

image
rschristian commented 9 months ago

Thanks for help, I was using this documentation

Being the Preact website, that's for Preact usage. Docs for the React integration are kept in this repo.

Someone's raised the idea of adding a banner to that page in the past to get React users back to the right place but I don't think anyone's worked on that yet.

But i don't get it. Now, however, a change in the value of the Signals variable triggers me to render all sibling and child components as if I were using useState.

Hm, not sure. I'm not familiar with the React integration. Indeed, neither the Babel plugin & useSignals seem to support the rendering optimization at the moment.

XantreDev commented 9 months ago

@rschristian In place dom updates via signals have never worked with react integration. But it realizable with custom components

rschristian commented 9 months ago

In place dom updates via signals have never worked with react integration

That doesn't sound right, and our docs indicate otherwise. Some usages did have issues depending on how they were themselves also using internals though (which is to be expected).

Wasn't skipping the VDOM of course but it should've been updating w/out rerenders.

XantreDev commented 9 months ago

It always worked like just signal value unpacking so parent component will rerender. https://github.com/preactjs/signals/blob/e42199ed4f00cc751cdf0f0b9de44c818a28c044/packages/react/src/index.ts#L33

But putting signals as JSX have been working

rschristian commented 9 months ago

Oh apologies, I had misread their example.

Yup, if not binding to a text node, rerenders are unavoidable on signal value change. All we can do is ask the React team for improvements here (though they're completely disinterested, so don't count on it).

GioOli7 commented 9 months ago

Thank you @rschristian and @XantreGodlike for the clarification

ElBaDaNoS commented 9 months ago

To clarify, are you suggesting that preact-signals are not functioning within React as anticipated?

rschristian commented 9 months ago

To clarify, are you suggesting that preact-signals are not functioning within React as anticipated?

Not sure if you're asking OP or me (I struck through my previous comment, as I had misread the provided example), but no, this should be working as intended.