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

[SolidJS] Browser Main Fields override Export Conditions #10323

Closed
7 tasks done
ryansolid opened this issue Oct 3, 2022 · 3 comments
Closed
7 tasks done

[SolidJS] Browser Main Fields override Export Conditions #10323

ryansolid opened this issue Oct 3, 2022 · 3 comments
Labels
p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)

Comments

@ryansolid
Copy link

ryansolid commented Oct 3, 2022

Describe the bug

Expected Behavior

When an export condition matches it doesn't use the "browser" main field to remap it just because "browser" was set.

Actual Behavior

In our case it matches on "worker" and then gets upgraded to the "browser" version unexpectedly.

Additional Information

solidjs/solid-start#263 Lays out the issue.

This issue isn't only unexpected but it makes things very difficult. We have 3 things trying to balance:

  1. Server vs Browser runtimes are different for SolidJS
  2. Cloudflare Worker environments are Server environments for us but most tools call them browser, understandably because of Service/Web Workers. That's ok we can control this with export conditions.
  3. Browser mainfield support is necessary for legacy tooling support. But its existence causes the work we do to map workers to be undone.

All these seem unmoveable. But perhaps main field overriding export conditions is not. The linked issues shows our attempts at trying to fix this. And backlinks relevant Github issues.

Reproduction

https://github.com/patdx/solid-start-graphql-cloudflare/tree/main/solid-with-standard-adapter

System Info

System:
    OS: macOS 12.6
    CPU: (8) arm64 Apple M1
    Memory: 97.66 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.16.0/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.16.0/bin/npm
  Browsers:
    Chrome: 105.0.5195.125
    Edge: 105.0.1343.53
    Firefox: 102.0.1
    Safari: 16.0

Used Package Manager

pnpm

Logs

No response

Validations

@ryansolid ryansolid changed the title Browser Main Fields override Export Conditions [SolidJS] Browser Main Fields override Export Conditions Oct 3, 2022
@patak-dev patak-dev added p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) and removed pending triage labels Oct 3, 2022
@JamesYeoman
Copy link

I have the same problem, except I'm not using SolidJS. I'm using React.

I've got my issue detailed in the discord discussions tab.

I'll copy some of the more important details here, as I don't expect everyone to have access to Discord.

Vite module resolution seemingly doesn't respect the exports package.json field

So BWIP-JS released V4, with an improved ES Module interface.

This paragraph is directly from the Discord thread

The only trouble is, Vite's module resolution isn't respecting the exports field in bwip-js's package.json. I was expecting Vite to resolve to line 8 of https://github.com/metafloor/bwip-js/blob/master/package.json (exports.browser.import), but it appears to be resolving to line 9 instead (exports.browser.require). Either that, or it's resolving to line 33 (the legacy browser field).

I then linked this image of mine
image which I generated with vite --debug 2>&1 | tee vite-log.txt.

Note that ESBuild's module resolution is spec-compliant. It correctly prioritises the exports map over the legacy browser field.

I was able to confirm that Vite was ignoring the exports map, by using yarn patch bwip-js to point the legacy browser field to the ESM build instead of the CJS/legacy build. With that patch, I was able to see that Vite was resolving the ESM file, which confirmed the fact that the legacy browser field was being resolved instead of the exports map.

I was originally using Vite v3, but I tried moving to Vite v4 and got the exact same problems, and was able to reproduce the above yarn patch hack.

I've made a reproduction repo, just in case it's helpful.

@JamesYeoman
Copy link

Any traction on this? It's a blocker for me to start using the Bundler module resolution in my tsconfig

@bluwy
Copy link
Member

bluwy commented Oct 2, 2023

I've discussed with @sapphi-red and we think this is working as expected. The browser field currently overrides the exports condition in many bundlers today. This shouldn't be the case if browser field is a plain string, e.g. "browser": "./my-lib.browser.js" since it's a lower-priority entrypoint specifier.

The object form of browser field isn't specifying an entrypoint, but instead the files/modules it should replace if you're in a browser or browser-ish environment (e.g. workers). So on contrary it would have higher priority.

Closing this as expected behavior.

@bluwy bluwy closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
None yet
Development

No branches or pull requests

4 participants