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

WMR does not handle the "browser" field of imported packages correctly #784

Open
2 of 3 tasks
Aeriit opened this issue Jul 27, 2021 · 0 comments · May be fixed by #874
Open
2 of 3 tasks

WMR does not handle the "browser" field of imported packages correctly #784

Aeriit opened this issue Jul 27, 2021 · 0 comments · May be fixed by #874
Labels
bug Something isn't working

Comments

@Aeriit
Copy link

Aeriit commented Jul 27, 2021

Describe the bug

Packages making use of the browser field in their package.json to replace certain imports with others when running in a browser context throw an error.

This is not an issue if the package defines esmodules, modern, module, ..., but occurs in the case of the axios package for example because they define only the browser and main in their package.json.

The issue also doesn't occur if the browser field is simply a string.

To Reproduce

  1. (Add the axios package) and import it in any file
  2. Run npm start (or yarn run start if that's more your cup of tea)
  3. Navigate to the page and see the error

Expected behavior

WMR bundles the package and handles the browser field correctly.

Bug occurs with:

  • wmr or wmr start (development)
  • wmr build (production)
  • wmr serve

Desktop (please complete the following information):

  • OS: Ubuntu 20.04 through WSL2
  • Browser: all
  • Node Version: 14.17.2
  • WMR Version: 3.4.1

Additional context

Link to the spec that isn't handled correctly: https://github.com/defunctzombie/package-browser-field-spec#replace-specific-files---advanced

The following code is causing the issue:

function resolveLegacyEntry(pkg, path) {
const entry =
_resolveLegacyEntry(pkg, {
browser: true,
fields: ['esmodules', 'modern', 'module', 'jsnext:main', 'browser', 'main']
}) || 'index.js';
return '/' + entry.replace(/^\.?\//, '');
}

resolve.exports's legacy (imported as _resolveLegacyEntry in this file) can also return a Record<string, string | false> due to the browser field, which is not handled - the code tries to call replace on what is possibly an object, causing it to throw an error in cases like these.

@Aeriit Aeriit added the bug Something isn't working label Jul 27, 2021
@Aeriit Aeriit changed the title WMR does not handle the "browser" field of packages correctly WMR does not handle the "browser" field of imported packages correctly Jul 27, 2021
@marvinhagemeister marvinhagemeister linked a pull request Sep 19, 2021 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant