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

Module main file name remains in cache, causing issues when main file is changed in newer versions #4332

Closed
julianduque opened this issue Dec 17, 2015 · 6 comments
Labels
module Issues and PRs related to the module subsystem.

Comments

@julianduque
Copy link
Contributor

Re-opening this issue here by @indutny request
Related: nodejs/node-v0.x-archive#8266
Reproduction case: https://gist.github.com/sbellone/3947632a42be73553abb

Module._pathCache and var packageMainCache do not play well when you switch between different versions of the same module. I suggest that the version (or the compressed package-url) should be included in the cache key

@akhoury
Copy link

akhoury commented Dec 17, 2015

+1
have you tested this on Node 5+

@cjihrig
Copy link
Contributor

cjihrig commented Dec 17, 2015

What exactly is the expected output of the linked reproduction script? With node 0.10.40 and npm 1.4.28, I get a crash "cannot find module async." However, with latest master and npm 3.3.12, the script runs to completion and prints winston version 0.6.2 and 0.7.2.

@akhoury
Copy link

akhoury commented Dec 17, 2015

I get a crash "cannot find module async."

which is exactly the problem. quoting the description of the test-case gist

It will fail because 'winston' 0.6.x uses 'async' 0.1 whereas 'winston' 0.7.x uses 'async' 0.2, which has changed its main file. Since the main file name is kept in cache in the variable packageMainCache (in module.js),

However,

with latest master and npm 3.3.12, the script runs to completion and prints winston version 0.6.2 and 0.7.2.

Which means it was fixed! yey!

@MylesBorins
Copy link
Contributor

this may need to be patched on v4.x

/cc @jasnell

@jasnell
Copy link
Member

jasnell commented Mar 22, 2016

Is this still an issue?

@akhoury
Copy link

akhoury commented Mar 22, 2016

nope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants