forceConsistentCasingInFileNames doesn't work for modules in node_modules

[OliverJAsh]

Bug Report

🔎 Search Terms

• forceConsistentCasingInFileNames
• node_modules

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about forceConsistentCasingInFileNames

⏯ Playground Link

Not available—requires node_modules folder

💻 Code

With forceConsistentCasingInFileNames enabled and fp-ts installed in node_modules:

src/struct.d.ts:

export const foo = 1;

// Expected error, but got none ❌
import * as xs1 from "fp-ts/lib/Struct";
// Expected error, but got none ❌
import * as xs2 from "fp-ts/lib/struct";
// Error as expected ✅
import * as xs3 from "./Struct";
// Error as expected ✅
import * as xs4 from "./struct";

🙁 Actual behavior

See code comments above.

🙂 Expected behavior

See code comments above.

[sheetalkamat]

I think this might be introduced in #44966

[fwienber]

We also have (even worse) trouble under Windows with fs.realpathSync.native() being used again since #44966.
On case-insensitive file systems, modules in node_modules are resolved to all-lower-case paths. tsc then tries to normalize that path, but if the original path contains upper-case letters and symlinks (Windows: junctions), fs.realpathSync.native() throws a "file not found" error, in contrast to fs.realpathSync(), which tolerates case differences and still normalizes the path.
The usage of realpath in sys.ts catches this error and returns the non-normalized path. Thus, in this case, normalization no longer happens!
In our concrete case, this leads to type errors in very subtle ways, because the same module is resolved in two different ways, so tsc regards a class imported from this module as different / incompatible.
Since this happens in a very large and complicated workspace, I am currently trying to set up a minimal example where this problem can be reproduced, which is unfortunately of course only possible under Windows and by using node_modules and junctions, so it cannot be a Playground example.

[fwienber]

News on this:

...but if the original path contains upper-case letters and symlinks (Windows: junctions)...

It turned out this was not the root of the problem, but instead, the issue occurs when TypeScript tries to normalize paths with more than 260 characters using fs.realpathSync.native(), despite Windows' 260 character file path limit having been switched off.

So the fix could be to fall back to fs.realpathSync() (which can deal with long file paths) either when the path to resolve is larger than 260 characters, or, more safely, when a "file not found" error is thrown by fs.realpathSync.native().

I could create a dedicated bug report and a PR for that fix, but it would be quite hard to implement a test or even a reproducer, because the error only occurs under such specific, OS-dependent conditions.

[sheetalkamat]

@fwienber thank you for getting to bottom of this.. Have PR #50306 up to fix this.. Can you try the build out and see how it is working out for you. Bot should comment on the PR when the build is ready for you to try it out. Thanks

Update: Build is available at #50306 (comment)

[fwienber]

Already fixed and arrived on main branch! 😲
I'm really disappointed as I was hoping do do my first PR for TypeScript... just kidding, I'm thrilled that this is already fixed!
Retesting the fix with our workspace, I can confirm that the problem disappears.

Just one caveat: The logic explicitly checking for the 260 character limit might fail when calling the utility method with a relative path. Such paths are resolved against the current working directory (process.cwd()), and if the resulting absolute path is not within the 260 character limit, the call would still throw an error. But I guess tsc never relies on the current working directory, i.e. uses no relative paths when calling realpath, so everything should be fine.

My proposed alternative fix, catching all "file not found" (e.code === 'ENOENT') errors and then falling back to non-native realpathSync, would be more robust and avoid the "magic number" 260 altogether, but have the downside of "programming by exceptions", slowing down the build process even more when dealing with many long paths. So all in all, I think checking the path length is the better choice.

Thank you again for your fast reaction and fix!

[fwienber]

One more thing, are you sure the original issue by @OliverJAsh is also resolved by this fix?
First, I assumed that our problem had to do with case sensitivity, too, but that turned out not to be the case. Maybe @OliverJAsh also used long file names, but it would be good to verify that.

[fwienber]

Process question: Will there be a 4.7.x release with this fix? Or only 4.8.x? Or maybe even only 4.9?

[sheetalkamat]

My mistake that i somehow thought the additional debugging that @fwienber did was related to original issue. Keeping this open as it is separate issue.

[sheetalkamat]

Just one caveat: The logic explicitly checking for the 260 character limit might fail when calling the utility method with a relative path. Such paths are resolved against the current working directory (process.cwd()), and if the resulting absolute path is not within the 260 character limit, the call would still throw an error. But I guess tsc never relies on the current working directory, i.e. uses no relative paths when calling realpath, so everything should be fine.

Yes the paths are expected to be full paths so this should never arise and in almost all cases if this is because of using relative path the issue would need fix somewhere else to make sure path is not relative to current directory

My proposed alternative fix, catching all "file not found" (e.code === 'ENOENT') errors and then falling back to non-native realpathSync, would be more robust and avoid the "magic number" 260 altogether, but have the downside of "programming by exceptions", slowing down the build process even more when dealing with many long paths. So all in all, I think checking the path length is the better choice.

Precisely why we are checking the path length and not handling exceptions so that we dont get perf hit on common scenarios.

[OliverJAsh]

@sheetalkamat I can still reproduce this in TS 4.8, with a slightly different test case. You will need to run yarn add fp-ts or npm install fp-ts.

import * as xs1 from "fp-ts/lib/struct";
// Error as expected ✅
import * as xs2 from "fp-ts/lib/Struct";

import * as xs3 from "fp-ts/struct";
// Expected error, but got none ❌
import * as xs4 from "fp-ts/Struct";

fp-ts/struct maps to fp-ts/lib/struct:

❯ cat node_modules/fp-ts/struct/package.json
{
  "main": "../lib/struct.js",
  "module": "../es6/struct.js",
  "typings": "../lib/struct.d.ts",
  "sideEffects": false
}

[sheetalkamat]

@OliverJAsh i dont think that case ever worked so that isnt regression per say.. This isnt something program can report easily either because package.json path's arent compared for file casing and typings for it points to lower case struct.d.ts so the resulting file will have correct casing.. So opening separate issue to track this