preactjs / signals

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

Component doesn't rerender after signal change (static setup) #303

Closed alexamy closed 1 year ago

alexamy commented 1 year ago

There is an example html page with counter code from tutorial. It doesn't work locally, but the same code from 'script' section works in REPL.

Steps to reproduce:

  1. Open https://codesandbox.io/s/preact-signal-bug-s91cfu
  2. Press '+' button few times

Expected: The count incrementing by 1 after '+' button press.

Actual: The count stays at 0 after '+' button press.

Note: There is a difference in a logged signal locally and in REPL. Locally property t is undefined, but in repl it equals to some object. (I assume it is a observer reference or something similar.)

alexamy commented 1 year ago

If I change signal to useSignal and move it in App component, it isn't working either. Example: https://codesandbox.io/s/preact-usesignal-bug-0c2x6q?file=/index.html

Got this error (you can see it in browser (not sandbox) console):

Uncaught TypeError: Cannot read properties of undefined (reading '__H')
    at d (hooks.js:5:14)
    at F (hooks.js:63:12)
    at useSignal (signals.js:178:10)
    at x.App [as constructor] ((index):32:21)
    at x.B [as render] (preact.js:287:15)
    at M (preact.js:210:51)
    at H (preact.js:136:7)
    at M (preact.js:212:265)
    at D (preact.js:291:103)
    at (index):43:5
alexamy commented 1 year ago

It works in app created by preact-cli, which uses Preact v10.11.3. After downgrading static html importmap with this version, signals are working!

https://codesandbox.io/s/preact-signal-bug-old-preact-s5teou?file=/index.html

marvinhagemeister commented 1 year ago

This is neither an issue with Preact or with signals. The code is not updating because you're loading multiple copies of Preact in that sandbox which are not aware of each other. Hooks require singletons to work behind the scenes (same in React) which doesn't play well with multiple copies of the framework being present.

Here is a fixed version of the codesandbox where only one copy of Preact is loaded: https://codesandbox.io/s/preact-signal-bug-forked-055v7z?file=/index.html

lucasmpr commented 1 year ago

Hello Guys, just had the same problem here.

Do you think we could put it somewhere on the docs?

I was really puzzled about it because nothing was working and there wasn't a single person commenting about it.

The example in preact site isn't working here either

https://preactjs.com/repl?example=todo-list-signals

jdgamble555 commented 1 year ago

Why is this closed? There is obviously a bigger issue here with signals not address in the latest version!

J

rschristian commented 1 year ago

Why is this closed? There is obviously a bigger issue here with signals not address in the latest version!

What issue is that?

marlun commented 1 year ago

I also clicked "Run in REPL" on the docs page and got confused when it didn't work.

image

The codebox from @marvinhagemeister do work but it's hard for a beginner to know that you need to do that for it to work. At least if you're not using a build step and just trying it out with esm.sh.

If I choose "Todo List (Signals" from the list on the REPL page, that doesn't work either:

image
fsterling1 commented 10 months ago

This is neither an issue with Preact or with signals. The code is not updating because you're loading multiple copies of Preact in that sandbox which are not aware of each other. Hooks require singletons to work behind the scenes (same in React) which doesn't play well with multiple copies of the framework being present.

Here is a fixed version of the codesandbox where only one copy of Preact is loaded: https://codesandbox.io/s/preact-signal-bug-forked-055v7z?file=/index.html

Hello, sorry the same issue for me, but I don't get it what exactly did you fix in the project so it's working. Could you please clarify? thank you

rschristian commented 10 months ago

@fsterling1

<script type="importmap">
    {
      "imports": {
-        "preact": "https://cdn.skypack.dev/preact@10.12.1",
+        "preact": "https://esm.sh/preact@10.12.1",
-        "preact/hooks": "https://cdn.skypack.dev/preact@10.12.1/hooks",
+        "preact/hooks": "https://esm.sh/preact@10.12.1/hooks?external=preact",
-        "@preact/signals": "https://cdn.skypack.dev/@preact/signals@1.1.3",
+        "@preact/signals": "https://esm.sh/@preact/signals@1.1.3?external=preact",
         "htm": "https://cdn.skypack.dev/htm@3.1.1"
      }
    }
  </script>

The important part is the ?external=preact (though I suppose it's also important to use a CDN that supports it). Without setting Preact as external, both preact/hooks and @preact/signals will drag down their own copies, giving us problems. When we set it to external, those modules will be forced to resolve preact through the import map we've written above, so there's only 1 copy.

fsterling1 commented 10 months ago

@fsterling1

<script type="importmap">
    {
      "imports": {
-        "preact": "https://cdn.skypack.dev/preact@10.12.1",
+        "preact": "https://esm.sh/preact@10.12.1",
-        "preact/hooks": "https://cdn.skypack.dev/preact@10.12.1/hooks",
+        "preact/hooks": "https://esm.sh/preact@10.12.1/hooks?external=preact",
-        "@preact/signals": "https://cdn.skypack.dev/@preact/signals@1.1.3",
+        "@preact/signals": "https://esm.sh/@preact/signals@1.1.3?external=preact",
         "htm": "https://cdn.skypack.dev/htm@3.1.1"
      }
    }
  </script>

The important part is the ?external=preact (though I suppose it's also important to use a CDN that supports it). Without setting Preact as external, both preact/hooks and @preact/signals will drag down their own copies, giving us problems. When we set it to external, those modules will be forced to resolve preact through the import map we've written above, so there's only 1 copy.

Thank you very much!, but what if I have react project, not Preact? where do I change this?

rschristian commented 10 months ago

In your import map, just swap out preact for react.

fsterling1 commented 10 months ago

In your import map, just swap out preact for react.

Thanks a lot for your prompt replies and patience), but sorry for an idiot questions, I'm kinda new in React, but need to get signal running. could you please tell exactly step by step - in what file should I write this Import map? I currently added it to Index.hml the top section, but nothing has changed.

rschristian commented 10 months ago

Just to check, are you using a build-less setup? If so, you can just copy from the code sandbox already provided, swapping out Preact for React.

fsterling1 commented 10 months ago

Just to check, are you using a build-less setup? If so, you can just copy from the code sandbox already provided, swapping out Preact for React.

yes, I copied the code sandbox and it worked for the given example (didn't change anything from 'preact'). But when I copy-paste import map into my html, and changing to react - doesn't work.

It has to look something like this?

<script type="importmap"> { "imports": { "react": "https://esm.sh/preact@10.12.1", "react/hooks": "https://esm.sh/preact@10.12.1/hooks?external=react", "@react/signals": "https://esm.sh/@preact/signals@1.1.3?external=react", "htm": "https://esm.sh/htm@3.1.1" } } </script>

rschristian commented 10 months ago

Er, you haven't fixed any of those URLs. You're still pulling Preact and referencing packages/paths that do not exist (react/hooks, @react/signals, etc). Might want to read up on import maps (MDN) but in the meantime:

<script type="importmap">
    {
      "imports": {
        "react": "https://esm.sh/react@18.2.0",
        "@preact/signals-react": "https://esm.sh/@preact/signals-react@1.3.6?external=react",
        "htm": "https://esm.sh/htm@3.1.1"
      }
    }
</script>

Edit:

The idea of an import map is to provide a value for a specifier, allowing you to avoid copy/pasting the same URL from file to file.

<script type="importmap">
    {
      "imports": {
        "my-fake-module": "https://esm.sh/my-fake-module",
      }
    }
</script>
<script type="module">
  import * as MyFakeModule from 'my-fake-module';
  ...
</script>

The issue in your case was a mismatch between specifier and CDN URLs, as well as the differing usage of Preact and React APIs -- Preact provides hooks from preact/hooks, but React provides them directly from react itself.

When swapping out Preact for React in the example, you need to switch the CDN URLs as well as the module specifiers for React use.

fsterling1 commented 10 months ago

Er, you haven't fixed any of those URLs. You're still pulling Preact and referencing packages/paths that do not exist (react/hooks, @react/signals, etc). Might want to read up on import maps (MDN) but in the meantime:

<script type="importmap">
    {
      "imports": {
        "react": "https://esm.sh/react@18.2.0",
        "@preact/signals-react": "https://esm.sh/@preact/signals-react@1.3.6?external=react",
        "htm": "https://esm.sh/htm@3.1.1"
      }
    }
</script>

Edit:

The idea of an import map is to provide a value for a specifier, allowing you to avoid copy/pasting the same URL from file to file.

<script type="importmap">
    {
      "imports": {
        "my-fake-module": "https://esm.sh/my-fake-module",
      }
    }
</script>
<script type="module">
  import * as MyFakeModule from 'my-fake-module';
  ...
</script>

The issue in your case was a mismatch between specifier and CDN URLs, as well as the differing usage of Preact and React APIs -- Preact provides hooks from preact/hooks, but React provides them directly from react itself.

When swapping out Preact for React in the example, you need to switch the CDN URLs as well as the module specifiers for React use.

I got it, thank you very much! I tried to copy paste your code with react proper links, changed the code for imports to react and @preact/signals-react, but still the value doesn't update on the screen (even in the example from sandbox - it's showing blank page)

rschristian commented 10 months ago

Sorry, forgot you'll need react/ as we use react/jsx-runtime. Here's a full example: https://stackblitz.com/edit/web-platform-yhfn66?file=index.html

And the revised map:

<script type="importmap">
    {
        "imports": {
          "react": "https://esm.sh/react@18.2.0",
          "react/": "https://esm.sh/react@18.2.0/",
          "react-dom": "https://esm.sh/react-dom@18.2.0",
          "@preact/signals-react": "https://esm.sh/@preact/signals-react@1.3.6?external=react",
          "htm": "https://esm.sh/htm@3.1.1"
        }
    }
</script>
fsterling1 commented 10 months ago

Sorry, forgot you'll need react/ as we use react/jsx-runtime. Here's a full example: https://stackblitz.com/edit/web-platform-yhfn66?file=index.html

And the revised map:

<script type="importmap">
    {
        "imports": {
          "react": "https://esm.sh/react@18.2.0",
          "react/": "https://esm.sh/react@18.2.0/",
          "react-dom": "https://esm.sh/react-dom@18.2.0",
          "@preact/signals-react": "https://esm.sh/@preact/signals-react@1.3.6?external=react",
          "htm": "https://esm.sh/htm@3.1.1"
        }
    }
</script>

Could you please show me on this example?

https://stackblitz.com/edit/stackblitz-starters-cmfkij?file=public%2Findex.html

rschristian commented 10 months ago

I just provided a full example, use it as a reference.

That Stackblitz you provided is a React app with build tooling. None of this is particularly relevant if you're using a build tool, hence why I asked earlier:

Just to check, are you using a build-less setup?

fsterling1 commented 10 months ago

I just provided a full example, use it as a reference.

That Stackblitz you provided is a React app with build tooling. None of this is particularly relevant if you're using a build tool, hence why I asked earlier:

Just to check, are you using a build-less setup?

ok, in case React app with build tooling what would be the solution?

marvinhagemeister commented 10 months ago

@fsterling1 We're more than happy to provide help regarding issues related to Preact, but we are not tech support for issues with other frameworks you may encounter. We do in general help with general web questions as well, but you're stretching it quite a bit here. If you're running into issues with React, then the React issue tracker is the proper place to ask for help about questions regarding React.

rschristian commented 10 months ago

In that case none of this has been relevant to this issue/thread, which is focused on situations without any build tools. It'd helped a lot had you provided a reproduction up-front if you weren't quite sure whether you were using a build tool or not.

As for usage w/ a build tool, it's really as simple as installing & importing from @preact/signals-react

Edit: I'll hide this whole conversation as it won't be useful for any future people looking for info

fsterling1 commented 10 months ago

sorry guys, won't be bothering you here.