Skip to content

Fix errors when importing the package in a DOMless environment (e.g. Node) #115

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 34 commits into from
May 22, 2025
Merged

Fix errors when importing the package in a DOMless environment (e.g. Node) #115

merged 34 commits into from
May 22, 2025

Conversation

benface
Copy link
Collaborator

@benface benface commented May 20, 2025

Third time's the charm? (original PRs: #109, #111)

benface and others added 27 commits May 20, 2025 12:31
@benface
Change exports "import" field to "default"
4b23f33 
This fixes the following error I'm getting when building my Next.js app that uses this package:

```
my-app:build: Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in /Users/benface/my-app/node_modules/style-observer/package.json
my-app:build:     at exportsNotFound (node:internal/modules/esm/resolve:314:10)
my-app:build:     at packageExportsResolve (node:internal/modules/esm/resolve:604:13)
my-app:build:     at resolveExports (node:internal/modules/cjs/loader:657:36)
my-app:build:     at Function._findPath (node:internal/modules/cjs/loader:749:31)
my-app:build:     at Function.<anonymous> (node:internal/modules/cjs/loader:1387:27)
my-app:build:     at /Users/benface/my-app/node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected][email protected]/node_modules/next/dist/server/require-hook.js:55:36
my-app:build:     at Function.resolve (node:internal/modules/helpers:145:19)
my-app:build:     at _resolve (/Users/benface/my-app/node_modules/.pnpm/[email protected]/node_modules/jiti/dist/jiti.js:1:246378)
my-app:build:     at jiti (/Users/benface/my-app/node_modules/.pnpm/[email protected]/node_modules/jiti/dist/jiti.js:1:249092)
```
@benface
default must be last
86f477e
@benface
Try to fix ReferenceError: document is not defined in SSR
1ae2575
@benface
Fix syntax errors
4a051f9
@benface
Fix “ReferenceError: requestAnimationFrame is not defined” in SSR
f29d21f
@benface
Another one
6e1d58f
@benface
Another one
41fc53d
@benface
Even more defensive
7c97a58 
This actually prevents an error, not when building in Next, but when running tests with Vitest and jsdom…
@benface
Address Lea’s feedback
34ed286
@benface @LeaVerou
Update src/util/detect-bugs/transitionrun-loop.js
8caa4f0 
Co-authored-by: Lea Verou <[email protected]>
@benface @LeaVerou
Update src/util/detect-bugs/transitionrun-loop.js
acc1363 
Co-authored-by: Lea Verou <[email protected]>
@benface
Tweaks
96bbd0a
@benface
Try to make package side-effect-free
0223ebf
@benface
Clean up
6e8a4e0
@benface
Oops
fb84913
@benface
Fix bug + make it closer to previous syntax
76189a2
@benface
Add "sideEffects": false to package.json
6bf6e01
@benface
Restore allowDiscrete const
b80e00d
@benface @DmitrySharabin
Update package.json
7a1fabc 
Co-authored-by: Dmitry Sharabin <[email protected]>
@benface @DmitrySharabin
Update gentle-register-property.js
bbf7b66 
Co-authored-by: Dmitry Sharabin <[email protected]>
@benface
Update JSDoc for consistency
3220e04
@benface
Make observe synchronous again
b04c8c0
@benface
Bugs.detectAll() => Bugs.detect() + return results
1c59520
@benface
export default class Bugs
9a0bccb
@benface
Keep behavior of defaulting bugs to true, instead of false
4cc580d
@benface
Remove redundant @private annotations
2bf1679
@benface
Add empty line between unrelated blocks of code
d9fd029
@netlify Netlify
Copy link

netlify bot commented May 20, 2025 •
edited
Loading

✅ Deploy Preview for style-observer ready!

Name Link
🔨 Latest commit 0603c60
🔍 Latest deploy log https://app.netlify.com/projects/style-observer/deploys/682f47d3b7d5cd00089d0358
😎 Deploy Preview https://deploy-preview-115--style-observer.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 project configuration.

@benface
Copy link
Collaborator Author

benface commented May 20, 2025

@LeaVerou @DmitrySharabin – The following issues from the previous PR are still unresolved:

Let me know if I'm missing anything, and if you want me to work on the above (I know @LeaVerou wanted to work on it too). Thank you!

@LeaVerou
Copy link
Owner

Thanks! @benface should I commit directly or send a PR against this branch?

@benface
Copy link
Collaborator Author

benface commented May 20, 2025

Feel free to commit directly! Thank you so much.

@LeaVerou LeaVerou mentioned this pull request May 21, 2025
Merged
@benface
Copy link
Collaborator Author

benface commented May 21, 2025 •
edited
Loading

@LeaVerou – Following your comment here, I made this commit to keep things simple for now... but this PR no longer removes side effects, does it? We're back to just fixing crashes when importing the package in Node (though in a much nicer way than in my original PR, thank you very much for your help). Should I remove "sideEffects": false from package.json and edit the PR's title, or shall we try to actually remove side effects by doing what you proposed with an #initGlobalContext() method? cc @DmitrySharabin

@LeaVerou
Copy link
Owner

@LeaVerou – Following your comment here, I made this commit to keep things simple for now... but this PR no longer removes side effects, does it? We're back to just fixing crashes when importing the package in Node (though in a much nicer way than in my original PR, thank you very much for your help). Should I remove "sideEffects": false from package.json and edit the PR's title, or shall we try to actually remove side effects by doing what you proposed with an #initGlobalContext() method? cc @DmitrySharabin

I’m fine with either of these options, up to you! 😊

@benface benface changed the title Remove side effects Fix errors when importing the package in a DOMless environment (e.g. Node) May 22, 2025
@benface
Copy link
Collaborator Author

benface commented May 22, 2025

@LeaVerou – I went with option A for now. Thank you! 🙏

@benface benface merged commit c54a1ff into main May 22, 2025
4 checks passed
@benface benface deleted the remove-side-effects branch May 22, 2025 15:53
@benface
Copy link
Collaborator Author

benface commented May 22, 2025

Oh no @LeaVerou @DmitrySharabin ... I'm so used to the big "Merge" button automatically squashing commits because that's the only setting we allow in my company. I'm so sorry that I spammed main like that. 😬 Might I recommend these settings under Settings > General?

CleanShot 2025-05-22 at 11 58 43@2x

@DmitrySharabin
Copy link
Collaborator

Actually, we do exactly that for our other projects. It's just slipped through some cracks here. 🙂
We can still squash some commits. Ensure if we can do that with merge commits, though.

@DmitrySharabin
Copy link
Collaborator

@benface, I updated the commit history and released a new version. 🥳
Could you please check if we have resolved all the issues you encountered?

@benface
Copy link
Collaborator Author

benface commented May 23, 2025

Yes @DmitrySharabin, v0.1.0 seems to be working very well for me, thank you very much! The only issue that affects my project at this point is #113, but it's minor and not a blocker. I will also be using #45 as soon as it exists, but I simply read the initial values myself for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeaVerou LeaVerou LeaVerou approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benface @LeaVerou @DmitrySharabin