Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

prinsss
Copy link
Contributor

@prinsss prinsss commented Sep 20, 2023

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) {

This fixes "useMemo undefined" error when using `useSignal` hook with UMD scripts.
@changeset-bot
Copy link

changeset-bot bot commented Sep 20, 2023

🦋 Changeset detected

Latest commit: 79efe32

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
Copy link

netlify bot commented Sep 20, 2023

Deploy Preview for preact-signals-demo ready!

Name Link
🔨 Latest commit 79efe32
🔍 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.

Copy link
Member

@rschristian rschristian left a comment

Choose a reason for hiding this comment

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

Damn, good spot. Not sure how I missed that.

@rschristian rschristian merged commit e2e19b7 into preactjs:main Sep 22, 2023
5 checks passed
@rschristian
Copy link
Member

Thanks!

@github-actions github-actions bot mentioned this pull request Sep 22, 2023
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.

2 participants