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

webpack should force case on windows #435

Closed
tcoopman opened this issue Aug 28, 2014 · 26 comments

Comments

@tcoopman
Copy link
Contributor

@tcoopman tcoopman commented Aug 28, 2014

I just had some strange errors after a typo in my require with react.

I had:

var React = require('React'); //instead of require('react')

On linux this will fail.
But as this is on windows this works, but later on this gave problems:

....
WARNING in ./~/react/lib/ReactServerRenderingTransaction.js
There is another module with a equal name when case is ignored.
This can lead to unexpected behavior when compiling on a filesystem with other case-semantic.
Rename module if multiple modules are expected or use equal casing if one module is expected.

WARNING in ./~/React/lib/ReactServerRenderingTransaction.js
There is another module with a equal name when case is ignored.
This can lead to unexpected behavior when compiling on a filesystem with other case-semantic.
Rename module if multiple modules are expected or use equal casing if one module is expected.

WARNING in ./~/react/lib/onlyChild.js
There is another module with a equal name when case is ignored.
This can lead to unexpected behavior when compiling on a filesystem with other case-semantic.
Rename module if multiple modules are expected or use equal casing if one module is expected.
....

Can webpack force the case of imports on windows, so the require fails?

@syranide

This comment has been minimized.

Copy link
Contributor

@syranide syranide commented Aug 28, 2014

Not really affected, but 👍, seems like you would never want the current behavior.

@sokra

This comment has been minimized.

Copy link
Member

@sokra sokra commented Aug 28, 2014

On linux this will fail.

Yep, therefore these big warnings ;)

@jhnns

This comment has been minimized.

Copy link
Member

@jhnns jhnns commented Sep 1, 2014

I think @tcoopman is right, it should also fail on Mac/Windows. Mixing cases as long as there are case sensitive file systems out there is a bad idea.

@cobbweb

This comment has been minimized.

Copy link

@cobbweb cobbweb commented Oct 21, 2014

Just had this issue and was pulling my hair out trying to figure out was going wrong. React was whinging that my components weren't components!

Turns out one of them had require('React') and that was causing all my issues. Internally React uses instanceof which was fails because it must have been returning different objects for React and react.

I was getting all the warnings like the the @tcoopman posted, but didn't know why I was getting them until I found this issue.

Would be good if this was more than a warning, and was more informative. Something to the effect of: You've imported a module named React and react which conflicts on case-insensitive filesystems yada yada. Would have known what I'd done and fixed it straight away.

@Swivelgames

This comment has been minimized.

Copy link

@Swivelgames Swivelgames commented Dec 9, 2014

I also ran into this issue, which made things a bit difficult.

I'm not sure how much extra work it'd be, but it would be nice if it forced case-sensitivity. That way, you're not spamming the user with warnings about something that could be detrimental to your application (using multiple instances of the same module).

Instead, you're throwing a fatal, and letting the user know that there is no such thing as a module called "React". However, there is a module called "react".

In the end, iirc, all NPM module names are strictly lowercase, so simply treating the modules the same may not be an issue... For instance, simply throwing a one-liner warning about the discrepancies in case while still treating them like the same module (which they will be, 99.99% of the time).

</two-cents> 😃

@Industrial

This comment has been minimized.

Copy link

@Industrial Industrial commented Jun 11, 2015

It doesnt matter if a module name is the same as another module name. It matters that their relative and absolute paths are different. If you stick it all in an object and use only the filename part, of course you get clashes. why doesn't webpack just do:

var webpackObject = {
  'some/path/to/my/module': function(){ /*myModule*/ },
  'some/path/to/my/other/Module': function(){ /*myModule*/ }
}

This is totally acceptable but for webpack it is not acceptable. That's a problem. :)

@axelson

This comment has been minimized.

Copy link

@axelson axelson commented Jul 3, 2015

+1 for getting more info into the error message. Currently I'm staring at:

WARNING in ../~/script-loader/addScript.js
There is another module with an equal name when case is ignored.
This can lead to unexpected behavior when compiling on a filesystem with other case-semantic.
Rename module if multiple modules are expected or use equal casing if one module is expected.

WARNING in ../~/Script-loader/addScript.js
There is another module with an equal name when case is ignored.
This can lead to unexpected behavior when compiling on a filesystem with other case-semantic.
Rename module if multiple modules are expected or use equal casing if one module is expected.

Which makes it difficult to track down which module is causing the issue. I'd rather not resort to running on Linux just to find the problem...

@axelson

This comment has been minimized.

Copy link

@axelson axelson commented Jul 3, 2015

I ended up doing a manual binary search on my modules and found I was calling require("Script!./myfile.js") instead of require("script!./myfile.js") which makes some sense given the warning but was not what I expected at all. I thought it was something that was being loaded through the script loader, not the script loader itself.

@Swivelgames

This comment has been minimized.

Copy link

@Swivelgames Swivelgames commented Jul 6, 2015

The biggest issue I see is that when these discrepancies exist, Webpack treats them as different modules, and loads both of them. We run into major issues because (1) we don't know the issue exists due to the warning/error messages, (2) we do NOT want multiple copies of the same module loaded and packed into our code. e.g. I don't want module 1.2 and Module 1.2 loaded, because that doubles the file size, transfer size, etc... because of that minor discrepancy.

Why not treat them the same (as it should), and simply throw a warning if the case is not lowercased? There's no reason why Webpack should be including and packing multiple instances of the same module+version combo in a single package.

@sokra

This comment has been minimized.

Copy link
Member

@sokra sokra commented Jul 7, 2015

Using the wrong casing is wrong, because there are case-sensitive filesystems, that won't find the file at all. This means using the wrong casing would break on cases-sensitive filesystems (i. e. production linux server).

It's better to emit an error in every case than to emit only an error on case-sensitive filesystems.

@Industrial

This comment has been minimized.

Copy link

@Industrial Industrial commented Jul 9, 2015

Don't design for the users that will not comprehend the difference between case sensitive and case insensitive. They need to learn the difference by making mistakes. Design it for the poor sysadmin/devops that is going to have to deploy their toys on a real system, imho.

@Swivelgames

This comment has been minimized.

Copy link

@Swivelgames Swivelgames commented Jul 13, 2015

@sokra @Industrial I can get on board with that. If we error out, instead of warning the user, than Webpack won't even get to the point of packaging the same module twice, which solves the issue.

The final issue we're left with is the current message's ambiguity, which does not facilitate a resolution. It's not a difficult correction to make, regardless of skillset and experience, so I can definitely agree that ERRORing out (as opposed to warning the user, which allows the process to continue) would be an effective means for ensuring resolution. However, the error message still definitely needs to be updated to be less ambiguous.

(1) Fatal instead of Warn (on all platforms)
(2) Disambiguate error/warning message

😄 👍

@istarkov

This comment has been minimized.

Copy link

@istarkov istarkov commented Oct 23, 2015

+1 (1) Fatal instead of Warn (on all platforms)

@bebraw bebraw added the bug label Nov 15, 2015
@KidsKilla

This comment has been minimized.

Copy link

@KidsKilla KidsKilla commented Dec 12, 2015

Friendly bump.

@vsych

This comment has been minimized.

Copy link

@vsych vsych commented Feb 18, 2016

+1 and on os x

@LeiZeng

This comment has been minimized.

Copy link

@LeiZeng LeiZeng commented Feb 23, 2016

+1 (1) Fatal instead of Warn (on all platforms)

on os x

It breaks the compiling eventually when try to deploy our project on Linux.

@Urthen

This comment has been minimized.

Copy link

@Urthen Urthen commented Jul 8, 2016

Since this bug seems to have been passed over by webpack core, we've got a plugin in the meantime to force case sensitivity: https://github.com/Urthen/case-sensitive-paths-webpack-plugin

It uses a fatal error whenever you try and import a module with the wrong case - even if you don't ever import the module with the correct case.

@bebraw bebraw added the webpack-3 label Jan 2, 2017
@webpack-bot

This comment has been minimized.

Copy link
Contributor

@webpack-bot webpack-bot commented Aug 11, 2017

This issue had no activity for at least half a year.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@TheLarkInn

This comment has been minimized.

Copy link
Member

@TheLarkInn TheLarkInn commented Aug 11, 2017

@Urthen Thanks for creating the plugin. Would you like to PR this into core as a feature now? Going to mark this as Help Wanted. (Pruning issue list).

@webpack-bot

This comment has been minimized.

Copy link
Contributor

@webpack-bot webpack-bot commented Feb 10, 2018

This issue had no activity for at least half a year.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@Swivelgames

This comment has been minimized.

Copy link

@Swivelgames Swivelgames commented Feb 15, 2018

@TheLarkInn @Urthen Any updates on this? Would be awesome to have this. :) This is still an issue, unfortunately.

@webpack-bot webpack-bot removed the inactive label Feb 15, 2018
@mc-zone

This comment has been minimized.

Copy link
Member

@mc-zone mc-zone commented Feb 22, 2018

I'd like to work with it. But how could we detect case wrong with high efficiently? It may bring performance decrease due to bunch of disk read.

Node v9.2 has add an implementation fs.realpath.native that can read real path with case-sensitive for a file. I think it performance maybe better than readdir iterative for all requests, but can't use below v9.2.

@mc-zone

This comment has been minimized.

Copy link
Member

@mc-zone mc-zone commented Feb 22, 2018

I've also add a resolve failed check in webpack/enhanced-resolve#137. Just triggered when request not found in CASE-SENSITIVE env and using readdir. Not sure how could we do it elegantly in CASE-INSENSITIVE env.

@Urthen

This comment has been minimized.

Copy link

@Urthen Urthen commented Feb 22, 2018

The (in)efficiency of my plugin is part of the reason I haven't decided to try and port it to Webpack. Right now the plugin works by, for each path, checking the contents of it's parent directory (which will, so far in practice, always return the correct casing even on case-insensitive systems) and seeing if one matches. This is then performed recursively up to the current working directory.

It has intelligent caching to prevent it from repeatedly checking the same information. It saves the contents of each directory, so when there are two or more files in the same directory, it only reads that directory's contents once.

In practice it generally reads all (Javascript source-containing) directories in the project once. It's not too inefficient, but it certainly isn't elegant. If we're OK with this behavior and want to add it in as an internal official optional plugin, I can take a look at that.

@webpack-bot

This comment has been minimized.

Copy link
Contributor

@webpack-bot webpack-bot commented Aug 24, 2018

This issue had no activity for at least half a year.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@webpack-bot

This comment has been minimized.

Copy link
Contributor

@webpack-bot webpack-bot commented Sep 8, 2018

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.

erchaves added a commit to mplusmuseum/mplusmuseum-collections-explorer that referenced this issue Nov 27, 2018
The hot-reloading was failing silently even though the rest of the app was working. It should have failed completely because the casing was wrong.

See also webpack/webpack#435
In that case mentioned above ^ it still works on windows but would fail on linux and gives a warning.  In this case, it worked on linux but should have forced a fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.