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: drop "node" and "browser" null export conditions #1941

Closed
wants to merge 1 commit into from

Conversation

jd-carroll
Copy link

Fixes #1809

@kettanaito
Copy link
Member

Hi, @jd-carroll. Thanks for opening this pull request.

It's not that simple though. This change on its own doesn't fix anything because it removes a valid export condition for those particular export paths. You aren't meant to use msw/node in the browser and msw/browser in Node.js. Those export conditions state precisely that.

I highly recommend following that GitHub issue for more context. The root cause for this issue is the tooling (Jest, Next, etc) is unable to handle export conditions properly. Those conditions are handled by the bundler but not all tools are configured to support multi-environment input, even if it's not explicitly restricted (like JSDOM pretending it runs in a browser, forcing browser module resolution onto you, which is a terrible mistake on their end).

We have discussed this internally with the team and are considering dropping the null export conditions in favor of placeholder modules that will simply throw in their stead. Similar to what client-only does for the use client directive in React. I am a bit skeptical about that approach though. Fixing the resolution error isn't the point. The point is to let the developers use the tools correctly. If something isn't meant to be used in a particular environment, it must fail fast. Failing on build time is faster than failing on runtime (which happens if we drop the export conditions and use placeholder modules).

Any input is welcome on this.

@kettanaito kettanaito changed the title Package exports should only contain valid environments fix: drop "node" and "browser" null export conditions Dec 27, 2023
@jd-carroll jd-carroll closed this Dec 31, 2023
@jd-carroll
Copy link
Author

Closing for now as the PR is old. Take a look at my comments in associated issue. If the updated patch fixes things then I'll open a new PR

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

Successfully merging this pull request may close these issues.

Jest : Cannot find module 'msw/node' from ...
2 participants