preactjs / signals

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

fix: add microbundle global for preact/hooks package #415

Closed prinsss closed 1 year ago

prinsss commented 1 year ago

Description

This fixes "useMemo undefined" error when using useSignal hook with UMD scripts.

The UMD builds of preact/hooks package will attach itself to the global window.preactHooks when using CDN, while @preact/signals is searching for window.hooks and cause this problem.

There is also some messages about this in microbundle's output log:

$ pnpm run build:preact

> pnpm _build --cwd packages/preact && pnpm postbuild:preact
> microbundle --raw --globals @preact/signals-core=preactSignalsCore "--cwd" "packages/preact"

No name was provided for external module 'preact/hooks' in output.globals ā€“ guessing 'hooks'

Bug Reproduction

Here is a minimal reproduction page demonstrating the issue.

https://stackblitz.com/edit/preact-signals-cdn

Uncaught TypeError: Cannot read properties of undefined (reading 'useMemo')
    at useSignal (signals.min.js:1:2667)
    at g.App [as constructor] (use-signal-broken.html:36:29)
    at g.N [as render] (preact.min.js:1:8510)
    at j (preact.min.js:1:6180)
    at x (preact.min.js:1:2071)
    at j (preact.min.js:1:6394)
    at O (preact.min.js:1:8631)
    at use-signal-broken.html:50:7

Expected Changes

The changes to the UMD build of @preact/signals package after applying this patch is as below:

--- signals-broken.min.js       2023-09-20 18:46:16
+++ signals-fixed.min.js        2023-09-20 18:46:14
@@ -11,7 +11,7 @@
                : i(
                                ((n || self).preactSignals = {}),
                                n.preact,
-                               n.hooks,
+                               n.preactHooks,
                                n.preactSignalsCore
                  );
 })(this, function (n, i, r, t) {
changeset-bot[bot] commented 1 year ago

šŸ¦‹ Changeset detected

Latest commit: 79efe32437784a2f7583fc727f9f99324289d11d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | --------------- | ----- | | @preact/signals | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify[bot] commented 1 year ago

Deploy Preview for preact-signals-demo ready!

Name Link
Latest commit 79efe32437784a2f7583fc727f9f99324289d11d
Latest deploy log https://app.netlify.com/sites/preact-signals-demo/deploys/650ad4404da9e9000857644a
Deploy Preview https://deploy-preview-415--preact-signals-demo.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

rschristian commented 1 year ago

Thanks!