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: Use subpath imports to choose Node or vanilla JS environment #88

Merged
merged 4 commits into from
Mar 13, 2024

Conversation

ukstv
Copy link
Contributor

@ukstv ukstv commented Feb 26, 2024

The changes here address #87

Note FYI: There is no indication of a package manager used. If using PNPM, which is slightly more strict than NPM, some of the transitive dependencies from aegir like @types/mocha, @types/node, mocha are considered to be unavailable. The PR has no related changes.

@ukstv ukstv changed the title chore: use subpath imports to delineate between node and vanilla JS environment Use subpath imports to choose Node or vanilla JS environment Feb 26, 2024
@ukstv ukstv changed the title Use subpath imports to choose Node or vanilla JS environment fix: Use subpath imports to choose Node or vanilla JS environment Feb 26, 2024
@stbrody
Copy link

stbrody commented Mar 6, 2024

FYI @achingbrain we've seen this come up a few more times with Ceramic users, would be great to get this in if possible.

Copy link

codecov bot commented Mar 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.34%. Comparing base (e6c1f80) to head (8dc30b2).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #88      +/-   ##
==========================================
+ Coverage   99.26%   99.34%   +0.07%     
==========================================
  Files          16       22       +6     
  Lines         547      612      +65     
  Branches      102      106       +4     
==========================================
+ Hits          543      608      +65     
  Misses          4        4              
Flag Coverage Δ
chrome 98.07% <100.00%> (+4.77%) ⬆️
node 98.20% <100.00%> (+8.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@achingbrain
Copy link
Owner

achingbrain commented Mar 7, 2024

@ukstv I've made a couple of changes:

  1. Import { Buffer } from node:buffer to make it easier for bundlers to polyfill if necessary (no need to mutate the global object)
  2. Add React Native overrides in package.json. It turns out React Native supports the exports map but not the imports one

Can you take a look at the updates please?

@achingbrain
Copy link
Owner

cc @wemeetagain for more eyeballs

@ukstv
Copy link
Contributor Author

ukstv commented Mar 8, 2024

It is amazing @achingbrain !! ❤️

@achingbrain achingbrain merged commit e43f1f4 into achingbrain:main Mar 13, 2024
21 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 13, 2024
## [5.0.3](v5.0.2...v5.0.3) (2024-03-13)

### Bug Fixes

* Use subpath imports to choose Node or vanilla JS environment ([#88](#88)) ([e43f1f4](e43f1f4))
Copy link

🎉 This PR is included in version 5.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@2color
Copy link

2color commented Mar 25, 2024

I'm pretty sure it's related, but since this got merged, I've been having problems using packages that depend on this package in Codesandbox with Parcel. example

The error I get:

Could not find module in path: 'uint8arrays/dist/src/util/#alloc' relative to '/node_modules/uint8arrays/dist/src/util/bases.js'

I suppose this is bundler dependent and I haven't tried to debug this too much, so any pointers would be helpful.

@ukstv
Copy link
Contributor Author

ukstv commented Mar 25, 2024

@2color Indeed, this is bundler-dependent. Parcel AFAIK still relies on explicit opt-in to enable subpath imports/exports. parcel-bundler/parcel#7840 (comment) and https://parceljs.org/features/dependency-resolution/#package-exports tell if you add

 "@parcel/resolver-default": {
    "packageExports": true
  }

to package.json, that should enable subpath imports/exports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants