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

no-hide-core-modules is based on an invalid assumption #69

Closed
feross opened this issue Mar 2, 2017 · 3 comments · Fixed by singapore/lint-condo#246
Closed

no-hide-core-modules is based on an invalid assumption #69

feross opened this issue Mar 2, 2017 · 3 comments · Fixed by singapore/lint-condo#246

Comments

@feross
Copy link
Contributor

feross commented Mar 2, 2017

You stated in #66 that:

If you depend on third-party modules which have the same name as a core module indirectly, you may use the third-party modules instead of core modules silently because of flattening node_modules. This might cause unintentional behaviors.

The behavior is actually the opposite. If you do npm install buffer and require('buffer') you get the core buffer module.

The only way to get the npm-installed version is with require('buffer/'). Although this looks hacky, the way that require works is locked, so this behavior won't ever change.

Not sure what you want to do with the rule now. It should probably be removed, since doing require('util') isn't dangerous, and it's not the user's fault if one of their dependencies does require('util/') in my opinion.

@mysticatea
Copy link
Owner

Thank you for this issue.

Oops, I had completely misunderstood.... 😢
Thank you very much for pointed out.

@feross
Copy link
Contributor Author

feross commented Mar 2, 2017

No problem 👍 We will be including this plugin in the next version of standard by the way. Great work!

@AdriVanHoudt
Copy link

I do follow the reasoning here and it seems valid. I just want to add that this rule has caught someone in our dev team that did npm i url. Seeing the comments above it would not have mattered but still some value ;) If node will always load internal modules first then removing the rule is 👌

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

Successfully merging a pull request may close this issue.

3 participants