Skip to content

fix: duplicate @preact/signals version#2964

Merged
marvinhagemeister merged 3 commits intomainfrom
signals-version
May 16, 2025
Merged

fix: duplicate @preact/signals version#2964
marvinhagemeister merged 3 commits intomainfrom
signals-version

Conversation

@marvinhagemeister
Copy link
Collaborator

We initialized new projects with @preact/signals@2.x but Fresh itself was using 1.x. There shall only ever be one version.

Kinda makes me wish Deno had some sort of peerDependencies.

Fixes #2962

@csvn
Copy link
Contributor

csvn commented May 16, 2025

Kinda makes me wish Deno had some sort of peerDependencies.

Yes, and version ranges, e.g. v1.x || v2.x.

@csvn
Copy link
Contributor

csvn commented May 16, 2025

I think tests will need to wait microtask/animation frame to match what Signals v2 had as breaking change. 🤔

Edit: @preact/signals@2.0.0

preactjs/signals#604 fea3e8d Thanks @JoviDeCroock! - Defer all DOM updates by an animation frame, this should make it so
that any previously synchronous DOM update will be instead delayed by an
animation frame. This allows Preact to first perform its own render
cycle and then our direct DOM updates to occur. These will now
be performed in a batched way which is more performant as the browser
is prepared to handle these during the animation frame.

This does impact how Preact based signals are tested, when
you perform a signal update, you'll need to wrap it in act. In a way
this was always the case, as a signal update that resulted in
a Preact state update would require it to be wrapped in act, but
now this is the norm.

Edit 2: Perhaps using act() from preact/test-utils can help, if there are signal changes in tests?

To execute state updates and effects synchronously, use the act function from preact/test-utils to wrap the code that triggers the updates.

Edit 3: Nvm, seems it was another issue, multiple different signal versions that may have been the issue 😅

@marvinhagemeister
Copy link
Collaborator Author

Upstream Deno issue for peerDependencies outside of npm packages denoland/deno#26199

@marvinhagemeister
Copy link
Collaborator Author

I think tests will need to wait microtask/animation frame to match what Signals v2 had as breaking change. 🤔

Nah, only the www e2e tests were failing. Turns out I forgot to increase the version number for signals in that.

"@fresh/plugin-tailwind": "../plugin-tailwindcss/src/mod.ts",
"@luca/esbuild-deno-loader": "jsr:@luca/esbuild-deno-loader@^0.11.0",
"@preact/signals": "npm:@preact/signals@^1.3.0",
"@preact/signals": "npm:@preact/signals@^2.0.4",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not possible to remove this dependency from here instead? Will that not use the same version as the root deno.json instead via workspaces?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah there is a PR for that #2900 . Was kinda hoping we could have support for workspace:* syntax similar to pnpm in Deno by now, but it looks like that is getting pushed on the back burner.

@marvinhagemeister marvinhagemeister merged commit 6ff0bec into main May 16, 2025
9 of 10 checks passed
@marvinhagemeister marvinhagemeister deleted the signals-version branch May 16, 2025 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v2.0] Bug: "Error: Cycle detected" when using @preact/signals@2.x

2 participants