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 empty polyfill if browser field is false #151

Merged

Conversation

markdalgleish
Copy link
Contributor

Please describe the changes this PR makes and why it should be merged:

Fixes remix-run/remix#7095 (comment).

The change you made to made to the browser field PR is actually the way I originally wrote it too, but in my local testing it caused the issue raised in the comment above. Sorry for not calling this out in the original PR.

As noted in the comment, I prefer the way you wrote it, but since it's a breaking change for Remix v1 consumers, I'm hoping we can hold off on this until a major release.

Status and versioning classification:

  • Code changes have been tested and working fine, or there are no code changes

@MichaelDeBoey
Copy link
Contributor

MichaelDeBoey commented Aug 23, 2023

Not sure if already handled, but since I was working on remix-run/web-std-io#43 (which added an exports.browser field) and came across this PR, I just wanted to let you know about the existence of the exports field (more info, see: https://nodejs.org/api/packages.html#exports).

If I'm correctly informed correctly (according to uuidjs/uuid#616 (comment)) the exports field is preferred over browser/main/module/types/...

@markdalgleish
Copy link
Contributor Author

@MichaelDeBoey This is a different concept. This is about swapping out package internals rather than its public API. In this case we only care if they're swapping out Node builtins, e.g.

{
  "browser": {
    "crypto": false
  }
}

Or

{
  "browser": {
    "crypto": "./polyfills/crypto.js"
  }
}

@imranbarbhuiya imranbarbhuiya merged commit 3946db8 into imranbarbhuiya:main Aug 24, 2023
7 checks passed
@imranbarbhuiya
Copy link
Owner

Thanks, I'll release in a few hrs

@markdalgleish markdalgleish deleted the browser-field-false-empty branch August 24, 2023 05:15
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.

Remix polyfills crypto in browser bundle
3 participants