preactjs / signals

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

@preact/signals-core v1.3.x fails on Fresh #389

Closed osddeitf closed 1 year ago

osddeitf commented 1 year ago

I write my website in Fresh, recently I upgraded Fresh to v1.3, along with all the Preact dependencies, including @preact/signals to v1.1.5 and @preact/signals-core to v1.3.1. Then I noticed the signals in my website is not working anymore. After trying downgrade @preact/signals and @preact/signals-core, I found out that @preact/signals-core version upto v1.2.3 would work. I also noticed the data for hydrations changed too:

v1.2.3

<script id="__FRSH_STATE" type="application/json">
{"v":[[{"requestURL":{"_f":"s","v":""}},{"requestURL":0},{}],[["responsive_container","old_logo","go-to-top","mainsearch","selector_english","searchfield","searchfield_input","oup_icons","responsive_hide_on_smartphone","responsive_hide_on_tablet","inputSuggestions","suggestionRow","suggestionCategory","suggestionDic","suggestionList"]]],"r":[[["0","0","requestURL"],["0","1","requestURL"]]]}
</script>

v1.3.1

<script id="__FRSH_STATE" type="application/json">
{"v":[[{"requestURL":{"_f":"s","v":""}},{"requestURL":{"_f":"s","v":""}},{}],[["responsive_container","old_logo","go-to-top","mainsearch","selector_english","searchfield","searchfield_input","oup_icons","responsive_hide_on_smartphone","responsive_hide_on_tablet","inputSuggestions","suggestionRow","suggestionCategory","suggestionDic","suggestionList"]]]}
</script>

As I compare the two JSON outputs, I think v1.3.x makes requestURL signal two different objects, while v1.2.3 make it one.

JoviDeCroock commented 1 year ago

Hmm, this might be related to adding Signal.prototype.toJSON, mind trying something like the following in the server-entrypoint of your application:

import { Signal } from '@preact/signals-core'

Signal.prototype.toJSON = undefined

EDIT: it looks like this got addressed in https://github.com/denoland/fresh/pull/1348

osddeitf commented 1 year ago

@JoviDeCroock I added the code you said, and it worked! But for your information, I'm using Fresh v1.3.0 which is just released yesterday, the PR was merged a month ago.

JoviDeCroock commented 1 year ago

@osddeitf I know that that PR got merged a month ago, I was mainly hinting at that maybe the detection there is running into an issue. The issue might be more appropriate on their repo 😅

osddeitf commented 1 year ago

If you think so, I'll ask there

marvinhagemeister commented 1 year ago

Closing in favour of https://github.com/denoland/fresh/issues/1488 because it's an issue in Fresh and not with Preact Signals.