-
Notifications
You must be signed in to change notification settings - Fork 7.3k
require() loads same module multiple times if case is different #6829
Comments
You linked to #5702, did you read it? Specifically, "would like to fix if there was a way"? This genuinely sucks, but it does not appear fixable. |
we had a discussion about this on the weekly call today, it may be worked around by normalizing the cache lookups |
@sam-github no, I just blindly link to closed issues 😉. Read @mathiasbynens's comments on #5702 - nobody has answered the question why. And nobody seems to revisit the closed issues, hence this new one. @tjfontaine awesome to hear that it might not be as impossible as previously thought! |
N.B. it's not an OS issue but a filesystem one. You can have filesystems with or without case sensitivity on Linux and OSX. |
I believe it's working as intended. If you have different case, it'll break on other platforms, so you shouldn't be doing that anyway. |
If your concern is "breaking on other platforms" then you should be able to see that this is clearly already broken. Let's say I'm now using this "feature" where var config1 = require('./Config'),
config2 = require('./config'),
config3 = require('./CONFIG');
config1.set("MEDIA_ROOT", "http://foo");
config2.set("MEDIA_ROOT", "http://bar");
config3.set("MEDIA_ROOT", "http://baz");
var app1 = createApp(config1),
app2 = createApp(config2),
app3 = createApp(config3); This would run on my OSX machine, and would create three separate config objects, with three separate What I'm suggesting is not to change how the inclusion of files from require works, but rather how the caching works. If I am on a case-insensitive FS, the above will work, but IMHO it shouldn't because var config1 = require('./Config'), //cached as file "__dirname/config.js"
config2 = require('./config'), //cached as file "__dirname/config.js"
config3 = require('./CONFIG'); //cached as file "__dirname/config.js"
config1.set("foo", "bar");
// now, the following is true:
// config1.get("foo") === config2.get("foo") === config3.get("foo") === "bar"; This would still break on a case-sensitive FS, but it would print out the standard "Cannot find module" error message which is easy to debug. On a case insensitive FS, no message is printed out when calling |
From the docs (emphasis mine):
The expectation then is that since "Config", "config", and "CONFIG" all resolve to the same file (config.js) on a case-insensitive FS, then each call should return "exactly the same object." So if multiple pathnames resolve to the same file, then there should only be one object created. I think that makes it pretty clear that there's no reasonable expectation it should work otherwise and that this is a bug... |
It is more global issue then i think before.. I can (and always did it) maintain all module.exports = { }; MyApp\Src\main.js var config = require("./config");
config.value="Hello world";
require("../Src/module1"); MyApp\Src\module1.js var config = require("./config");
console.log(config.value); Do you see any problems with case? No.. user 1 calls it:
Work nice although user 1 use 'wrong' cases for user 2 calls it:
no comments... ( |
May be it possible to use normalized names as keys for cache lookups? var key = filename;
if (process.platform === 'win32' ) { key = key.toLowerCase(); }
var cachedModule = Module._cache[key];
....
Module._cache[key] = module; |
It's better if it fails right away then if it will be failing sometimes in the future depending on the filesystem. |
@rlidwka you don't undestanding me. Problem not in application. Application absolutely correctly (or you can point to me where an error?). But nodejs give a chance to an user on Windows break it if user type in CLI 'wrong' case path. Sometimes user will type 'correct case' path and application will work fine. Such strange behavior cost me today about hour of debugging for little app. |
+1 In my case the problem is within node (11.10 x64) on Windows. I load a module with relative paths
The problem is that i cannot tell what module is loaded each time. |
+1 Another variety of this issue is that the npm registry is case sensitive: http://www.npmjs.org/package/faker Are two different modules. This means that on a case-sensitive OS you can require('Faker') and have it return an entirely different module on a case-insensitive system (e.g. if 'faker' is at a more prominent position in the search path). |
This commit fixes an issue where the SDK would not correctly load with `require("aws-sdk")` on a Windows machine running Node.js 0.11.x. This error is caused by the following two behavioral changes in Node.js: * nodejs/node-v0.x-archive#6829 * nodejs/node-v0.x-archive#7031 In essence, Node.js `require()` calls return different objects for differently cased filenames, even on case-insensitive file systems. This, coupled with the fact that `path.join()` lowercases the drive letter in the path, causes the SDK to try to load core.js two separate times, creating two objects that do not have the correct properties attached. See #303 for more information.
We had this issue in the aws-sdk due to the same behavior change @Volox reported. |
Closing the issue? |
... @indutny was this resolved??? Or is it a wontfix for some reason? |
I guess not :) cc @orangemocha |
Would be fixed by #6774 |
Main issue in here is the strange behaviour of fs.realpath. In many platforms realpath function actually gets the filename with correct case in case insensitive systems. But node does not. I'll bring this to io.js too. Maybe they decide to fix this. |
@orangemocha ... any further thoughts on this one? |
There is a PR that has been open for a while but never made it through. #6774 I will revisit that PR, and build the case for it again. I will probably bring the PR over to io.js. If it gets approved there, we can decide if it makes sense to backport it. |
Ok, will mark this as defer-to-converge for now then. |
PR #8145 addresses this issue. |
tl;dr - even though modules are frozen, a warning message could help debug some frustrating debug issues related to
require
's lack of case sensitivityAt the risk of beating a dead horse... #5702, #6000, #2621
I just ended up debugging an issue for 2 hours (on OSX), turns out
require
's lack of case sensitivity was the problem. Luckily I follow a naming convention and I spotted the capitalized filename in my own stack trace. Other than that, there was no clue as to what was going on. (FWIW, one file wasrequire
ing another using TitleCase instead of lowercase - this created two copies of the same module, which contained shared config settings.)require
should know on certain OS's that "./module.js" and "./Module.js" actually point to the same file and not create a separate instantiation of the module being required. IMHO, it should be case-sensitive so code is transportable. But at the very least, it could error/warn, or load in files in accordance with the OS, but it should store cached copies/references to the data loaded in based upon the actual file name found.Kind of a contrived example, but IMHO the following should either throw an error or work as expected, right now it simply breaks:
Since the second
cfg
was required using different case, a second object is returned and the "db" property hasn't been set and the app won't connect to the database.The text was updated successfully, but these errors were encountered: