Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

require() loads same module multiple times if case is different #6829

Closed
jessefulton opened this issue Jan 9, 2014 · 23 comments
Closed

require() loads same module multiple times if case is different #6829

jessefulton opened this issue Jan 9, 2014 · 23 comments

Comments

@jessefulton
Copy link

tl;dr - even though modules are frozen, a warning message could help debug some frustrating debug issues related to require's lack of case sensitivity

At 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 was requireing 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:

//config.js
module.exports.set = function(key, val) {/* */}
module.exports.get = function(key) {/* */}
//file1.js
var cfg = require("./config");
cfg.set("db", process.env.DB_CONNECTION_STRING);
//file 2.js
var cfg = require("./Config"); // <-- note capital 'C', this creates a NEW object, not the shared one as expected
var app = createApp();
app.connect(cfg.get("db")); // null

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.

@sam-github
Copy link

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.

@tjfontaine
Copy link

we had a discussion about this on the weekly call today, it may be worked around by normalizing the cache lookups

@jessefulton
Copy link
Author

@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!

@OrangeDog
Copy link

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.

@rlidwka
Copy link

rlidwka commented Jan 11, 2014

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

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.

@jessefulton
Copy link
Author

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 requireing a module with a different case on a case-insensitive FS creates a new instantiation of that module.

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 MEDIA_ROOT properties, which I could pass to three separate "apps." However, as soon as I deploy to one of my shared *nix environments with case-sensitive FS, then it would break (as expected.)

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 require should cache the included modules based upon their actual filename.

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 require with different capitalizations of the same module - that is a problem.

@jessefulton
Copy link
Author

From the docs (emphasis mine):

Modules are cached after the first time they are loaded. This means (among other things) that every call to require('foo') will get exactly the same object returned, if it would resolve to the same file.

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...

@litmit
Copy link

litmit commented Jan 15, 2014

It is more global issue then i think before..

I can (and always did it) maintain all require paths in my modules case consistent.
But how I can control my users? Meanwhile user may invoke my node app using 'invalid' case and this break my application in unpredicable way!!!
Consider next example:
MyApp\Src\config.js

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..
Ok, let try my cool MyApp on Windows.

user 1 calls it: node myApp\Src\Main.js
Output:

Hello world

Work nice although user 1 use 'wrong' cases for main.js and MyApp

user 2 calls it: node MyApp\src\main.js
Output:

undefined

no comments... (

@litmit
Copy link

litmit commented Jan 15, 2014

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;

@rlidwka
Copy link

rlidwka commented Jan 15, 2014

Meanwhile user may invoke my node app using 'invalid' case and this break my application in unpredicable way!!!

It's better if it fails right away then if it will be failing sometimes in the future depending on the filesystem.

@litmit
Copy link

litmit commented Jan 15, 2014

@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.
Sometimes user will type 'incorrect case' path and application will work in some strange manner.

Such strange behavior cost me today about hour of debugging for little app.

@Volox
Copy link

Volox commented Jan 17, 2014

+1

In my case the problem is within node (11.10 x64) on Windows. I load a module with relative paths ../core and inspecting require.cache I got both entries:

d:\path\to\node\module\core\index.js
D:\path\to\node\module\core\index.js

The problem is that i cannot tell what module is loaded each time.

@ashaffer
Copy link

+1

Another variety of this issue is that the npm registry is case sensitive:

http://www.npmjs.org/package/faker
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).

lsegal added a commit to aws/aws-sdk-js that referenced this issue Jun 19, 2014
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.
@lsegal
Copy link

lsegal commented Jun 19, 2014

We had this issue in the aws-sdk due to the same behavior change @Volox reported. require('./foo') returns a different object from require(path.join(__dirname, 'foo')) because of the related change reported in #7031. This causes problems when you're writing code that expects those require calls to return the exact same object (like, say, a config object that's re-used and mutated).

@indutny
Copy link
Member

indutny commented Jul 31, 2014

Closing the issue?

@indutny indutny closed this as completed Jul 31, 2014
@jessefulton
Copy link
Author

... @indutny was this resolved??? Or is it a wontfix for some reason?

@indutny indutny reopened this Jul 31, 2014
@indutny
Copy link
Member

indutny commented Jul 31, 2014

I guess not :) cc @orangemocha

@orangemocha
Copy link
Contributor

Would be fixed by #6774

@ghost
Copy link

ghost commented Feb 8, 2015

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.

@jasnell
Copy link
Member

jasnell commented May 28, 2015

@orangemocha ... any further thoughts on this one?

@orangemocha orangemocha self-assigned this May 29, 2015
@orangemocha
Copy link
Contributor

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.

@jasnell
Copy link
Member

jasnell commented Jun 3, 2015

Ok, will mark this as defer-to-converge for now then.

@FransGH
Copy link

FransGH commented May 14, 2016

PR #8145 addresses this issue.

@Trott Trott closed this as completed Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests