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

Don't use browser versions of libraries if they are installed separately #1657

Closed
TuurDutoit opened this issue Jul 3, 2018 · 10 comments
Closed

Comments

@TuurDutoit
Copy link

🐛 bug report

Browser versions of common libraries (like events for example) are used, even when they are explicitly installed with NPM.

🎛 Configuration (.babelrc, package.json, cli command)

I'm working on a pretty simple Vue.js project.

My .babelrc is as simple as it gets: { "presets": ["env"] }

My package.json (only relevant bits):

{
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1",
    "start": "parcel --open index.html",
    "build": "parcel build index.html",
    "clean": "rm -rf dist && rm -rf .cache",
    "dev": "npm run clean && npm run start"
  },
  "dependencies": {
    "events": "^3.0.0"
  }
}

I start Parcel with npm run dev

🔦 Context

Specifically for my use case, I want to use the standard EventEmitter in my code, like so: import EventEmitter from 'events';. This will use a browser implementation, instead of the built-in one from Node.js (as I'm building for the browser). So far so good.
However, I want to use the eventNames() method, which was added in Node.js v6.0.0, and is supported in modern versions of the events module/shim (at least v3.0.0, maybe even earlier). Apparently, the events implementation Parcel uses (provided by node-libs-browser) is quite outdated (v1.x.x, I think), and doesn't support this new feature.

🤔 Expected Behavior

To resolve this issue, I installed events@3.0.0 explicitly with NPM, hoping it would use that one, over the outdated version it used out-of-the-box.

😯 Current Behavior

Apparently, it doesn't. It still uses the old version from node-libs-browser.

💁 Possible Solution

Ideally, if a package is installed with NPM, Parcel should always choose that one, instead of using the browser shims.

🌍 Your Environment

Software Version(s)
Parcel 1.9.4
Node 9.11.1
npm/Yarn 5.6.0
Operating System Mac OS 10.13.5
@devongovett
Copy link
Member

Maybe you could make a PR to https://github.com/webpack/node-libs-browser to upgrade it?

@TuurDutoit
Copy link
Author

Of course, and I will. But still, this behaviour is quite weird, and it could have allowed me to work around the outdated node-libs-browser package

@goto-bus-stop
Copy link

goto-bus-stop commented Jul 4, 2018

node-libs-browser seems to not want to do a major bump yet, idk why. maybe it's just that nobody has time to look at it.
FWIW in the mean time you can do import EventEmitter from 'events/' to force module resolution to use your locally installed version instead of the core module shim. In browserify you can do -r events/:events to replace the core events module shim with a locally installed version, I don't know if Parcel has anything similar.

e; I guess you can use this babel plugin: https://www.npmjs.com/package/babel-plugin-module-resolver

@TuurDutoit
Copy link
Author

Thank you, that worked!

@DeMoorJasper DeMoorJasper added the ✖️ Non-Parcel bug Bugs related to dependencies or plugins label Jul 11, 2018
@icopp
Copy link

icopp commented Jul 20, 2018

My personal preference with this would be for Parcel to not include node-libs-browser. If I want node shims I'll use them myself, and the docs don't list it all, so taking over imports in this way is an unpleasant surprise.

@goto-bus-stop
Copy link

goto-bus-stop commented Jul 20, 2018

a lot of npm modules depend on node core modules, particularly events, assert, and util, and they don't list them in dependencies. failing to bundle valid npm modules would also be an unpleasant surprise.

(e; i agree explaining it in docs would be helpful.)

@icopp
Copy link

icopp commented Jul 20, 2018

a lot of npm modules depend on node core modules, particularly events, assert, and util, and they don't list them in dependencies. failing to bundle valid npm modules would also be an unpleasant surprise.

Wouldn't it be better to have it be an extension of the auto-install behavior, instead of silently bundling an out-of-date collection of modules? Adding specific packages as project dependencies as necessary would avoid issues like this one and also be much more explicit to users.

@TuurDutoit
Copy link
Author

That actually makes a lot of sense!

@tinchoz49
Copy link

If parcel relies in the deprecated node-libs-browser could be a nice thing to have an updated node-libs-browser that belongs to the parcel project. There are modules like path-browserify and buffer that we could update for the next major version of parcel and provide a better portability between node and the browser.

@mischnic mischnic added 🙋‍♀️ Feature and removed ✖️ Non-Parcel bug Bugs related to dependencies or plugins labels Oct 17, 2019
@mischnic
Copy link
Member

We don't use the node-libs-browser-approach anymore: the polyfill libraries are now (auto)installed into the project (so they're listed in the dependencies alongside parcel). So you can control the version now.

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

No branches or pull requests

7 participants