preactjs / signals

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

Creating a new deployment in PM2 breaks a simple Signal app #581

Closed saeho closed 2 days ago

saeho commented 5 days ago

Environment Using: @preact/signals-core v1.6.1 @preact/signals v1.2.3

With: Deno Fresh

Describe the bug I'm trying to figure out if this bug is from Deno/Fresh or Preactjs Signals.

There's 2 bugs here.

First, creating a very simple Signal app and then doing a new deployment using PM2 and serving the main.ts completely breaks the app for existing users until they refresh the browser.

Second, without any new deployments, using Signal and then doing a simple mouse over (to trigger the usage of Signal) and then attempting to route to another page breaks the app completely.

To Reproduce https://github.com/saeho/fresh-signal-bug

Follow steps mentioned above.

Related issue https://github.com/denoland/fresh/issues/2562

rschristian commented 2 days ago

The error you display in fresh#2564 (__H) is from loading multiple copies of Preact at once.

Multiple copies of Preact will indeed break Preact, this is expected. It's not related to signals.

You probably have a bug in your import map.

saeho commented 2 days ago

The error you display in fresh#2564 (__H) is from loading multiple copies of Preact at once.

Multiple copies of Preact will indeed break Preact, this is expected. It's not related to signals.

You probably have a bug in your import map.

This is my import map: https://github.com/saeho/fresh-signal-bug/blob/main/deno.json

I imported "preact" at 10.19.6 and "preact/" and 10.19.6 so I can import "preact" in the app and also other things like "preact/compat" from the app too. But I made sure to use same version.

Am I doing that wrong?

Can you advise please?

saeho commented 2 days ago
"preact": "https://esm.sh/preact@10.19.6",
"preact/": "https://esm.sh/preact@10.19.6/",
"@preact/signals": "https://esm.sh/*@preact/signals@1.0.1",
"@preact/signals-core": "https://esm.sh/*@preact/signals-core@1.0.1",

My minimal code repo doesn't import preact or preact/signals or preact/signals-core in any way other than this import map.

Both "preact" and "preact/" are required otherwise app can't run (preact/hooks can't be imported without the trailing "/" import.

@preact/signals and @preact/signals-core are both required, and I tested with 1.0.3/1.0.1 (latest versions) and I also tested with matching 1.0.1/1.0.1 even though I don't think you meant those versions have to match.

There are no other preacts imported in any other way (such as manual) anywhere in the minimal reproduceable repo.

Could you please advise where else a different version of preact might be imported from?

JoviDeCroock commented 2 days ago

@saeho look at your network tab and you'll quite quickly be able to observe what you are importing and what versions they are at, without that hint from your side we can't really help you.

My assumption would be that you need this

"preact": "https://esm.sh/preact@10.19.6",
"preact/hooks": "https://esm.sh/preact@10.19.6/hooks",
"@preact/signals": "https://esm.sh/*@preact/signals@1.0.1",
"@preact/signals-core": "https://esm.sh/*@preact/signals-core@1.0.1",
saeho commented 2 days ago

@saeho look at your network tab and you'll quite quickly be able to observe what you are importing and what versions they are at, without that hint from your side we can't really help you.

My assumption would be that you need this

"preact": "https://esm.sh/preact@10.19.6",
"preact/hooks": "https://esm.sh/preact@10.19.6/hooks",
"@preact/signals": "https://esm.sh/*@preact/signals@1.0.1",
"@preact/signals-core": "https://esm.sh/*@preact/signals-core@1.0.1",

In Deno, manually importing hooks like that is not necessary, but I did it your way anyways, and still got the same bug.

I actually think this bug is probably not on preact side but rather on the Deno Fresh framework side. Regardless, I'm so grateful you're replying to me right now (lol).

Can you tell me how I can see the version? Because in the network tab, all I see for signals.js is (this is for production, but similar for development as well without any mention of versions) ...

import {b as o} from "./chunk-ZKAIFXSE.js";
import "./chunk-FEDUQ665.js";
export {o as signal};

This bug only happens when an existing user with older build of the app clicks a link to a "newer" version of the app (after I rebuild and redeploy). This is the case because Deno Fresh is SSR, not an SPA, so when a new build is deployed, I assume preact/Signal is getting re-built too so this isn't a "different version" issue but "different build" issue--

if you can confirm that's the source of the bug, that would explain everything. Can you please confirm?

If this is all true, do you think that there's anything I or the Deno Fresh team can do to fix this?

rschristian commented 2 days ago

This bug only happens when an existing user with older build of the app clicks a link to a "newer" version of the app (after I rebuild and redeploy).

In that case, perhaps it's an issue with your user getting the updated import (for signals) but not the updated map which dedupes Preact. For now, I'd say this appears to be a Fresh issue.

Can you add @preact/signals to your import map now, and roll out usage of it (signals) later? A two-step approach might help you get around this, else, telling users to refresh their page is hardly a big ask.

saeho commented 2 days ago

This bug only happens when an existing user with older build of the app clicks a link to a "newer" version of the app (after I rebuild and redeploy).

In that case, perhaps it's an issue with your user getting the updated import (for signals) but not the updated map which dedupes Preact. For now, I'd say this appears to be a Fresh issue.

Can you add @preact/signals to your import map now, and roll out usage of it (signals) later? A two-step approach might help you get around this, else, telling users to refresh their page is hardly a big ask.

After reading everyone's messages above, yeah, I think so. I think this is a Fresh issue.

Deno Fresh is SSR, so I don't think there's any way I can seamlessly continue the experience between two routes after I re-build (because it's SSR, the URL to minified/bundled Signal library will be different between each builds).

I'm currently using web sockets to force refresh every client's browser whenever there's an update. It's not an ideal solution, but that's all I can do right now.