I don't think a proxy is the right way to fix it - it slows down all access to the cache which is on a hot path. If we want to do this I suspect this is beyond just the cache - the keys are computed by Module._resolveFilename and are attached to the modules as module.filename so that needs to be modified for consistency. Also, the cache is shared by the ESM loader, so something needs to be done for ESM as well for it to be consistent.

Thank you for the review.
I couldn't modify the function Module._resolveFilename because the require.cache variable can be used by the user directly. Here you can see.

Since the variable can be used without any get/set functions, I thought that overloading the operations could fix this issue. Then, I found Proxy and used it. You can see the benchmark results below:

                                          confidence improvement accuracy (*)   (**)  (***)
module\\module-loader-circular.js n=10000                -0.02 %       ±2.86% ±3.81% ±4.97%

I'm open to suggestions.

require.cache is mostly meant to be used with a filename previously resolved by Node.js (which is usually from Module._resolveFilename()). Only modifying Module._cache leads to inconsistency if users then try to use the mod.filename or __filename somewhere, or try to use it together with ESM (e.g. importing a URL constructed from that filename).

module-loader-circular.js isn't a very suitable benchmark because it deletes from the module cache, which is already not a fast path; You'd need to benchmark the common fast path where a module gets re-required and actually hitting the cache.

I've modified the benchmark test by applying the following patch:

diff --git a/benchmark/module/module-loader-circular.js b/benchmark/module/module-loader-circular.js
index db382142c2e..e712f4bf0fa 100644
--- a/benchmark/module/module-loader-circular.js
+++ b/benchmark/module/module-loader-circular.js
@@ -26,6 +26,11 @@ function main({ n }) {
     require(bDotJS);
     delete require.cache[aDotJS];
     delete require.cache[bDotJS];
+
+    require(aDotJS);
+    require(bDotJS);
+    require(aDotJS);
+    require(bDotJS);
   }
   bench.end(n);
 

The result of this benchmark is below.

                                          confidence improvement accuracy (*)   (**)  (***)
module\\module-loader-circular.js n=10000                -1.55 %       ±4.69% ±6.25% ±8.15%

Do you think this test is sufficient for benchmarking?

You wrote about inconsistency but I couldn't quite understand your concern. Could you please give me an example or point me to an existing test?
I would appreciate your help if you have something in your mind to fix this inconsistency.

The difference comes more from the delete, so they should be removed to check the impact on cache hits.

You wrote about inconsistency but I couldn't quite understand your concern. Could you please give me an example or point me to an existing test?

For example, Object.keys(require.cache).includes(require.resolve('TEST')) will not be the same as !!require.cache[require.resolve('TEST')], if the resolution is not updated (the key can also be just __filename from some module, or any other place to get a resolved file name). Also, import is still case-sensitive, which doesn't seem right if only require becomes case-insensitive.

Thank you for your detailed response. I completely understand your concern. I'll do my best to explore potential solutions to this problem and ensure consistency. After that, we can discuss and decide on the best course of action.

After further investigation, I found the place where the imported modules are cached. Then, I made import case insensitive using the attached patch. However, this test failed. As shown, String objects are not accepted in Node.js, which suggests that making imports case-insensitive may not be very practical.

Considering all these factors, I believe that it may not be worthwhile to pursue making both require and import case-insensitive.

diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js
index 0040ff5f5d1..b524bec1488 100644
--- a/lib/internal/modules/esm/module_map.js
+++ b/lib/internal/modules/esm/module_map.js
@@ -91,7 +91,7 @@ class LoadCache extends SafeMap {
   get(url, type = kImplicitTypeAttribute) {
     validateString(url, 'url');
     validateString(type, 'type');
-    return super.get(url)?.[type];
+    return super.get(url.toLowerCase())?.[type];
   }
   set(url, type = kImplicitTypeAttribute, job) {
     validateString(url, 'url');
@@ -107,15 +107,15 @@ class LoadCache extends SafeMap {
     }) in ModuleLoadMap`);
     const cachedJobsForUrl = super.get(url) ?? { __proto__: null };
     cachedJobsForUrl[type] = job;
-    return super.set(url, cachedJobsForUrl);
+    return super.set(url.toLowerCase(), cachedJobsForUrl);
   }
   has(url, type = kImplicitTypeAttribute) {
     validateString(url, 'url');
     validateString(type, 'type');
-    return super.get(url)?.[type] !== undefined;
+    return super.get(url.toLowerCase())?.[type] !== undefined;
   }
   delete(url, type = kImplicitTypeAttribute) {
-    const cached = super.get(url);
+    const cached = super.get(url.toLowerCase());
     if (cached) {
       cached[type] = undefined;
     }

As shown, String objects are not accepted in Node.js, which suggests that making imports case-insensitive may not be very practical.

I am not sure what's the correlation between the two? The test you are pointing to is the loader test which is an experimental feature for customizing the loader, and it's the customization that is returning String objects. The internal loader can do whatever coercion necessary to make it work.

Is StringPrototypeToLowerCase the right way to convert the paths to be case-insensitive on Windows? String.prototype.toLowerCase follows the unicode case-folding rules, https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.tolowercase , I can't really find proper documentation about the case-folding rules of Windows so far (it seems FS-dependent), but my experience dealing with Windows tells me it's probably something that predates unicode (surely they had this before 1991, and Windows tends to be very backwards-compatible) so it's unlikely to be consistent with the output of String.prototype.toLowerCase().

From the Internet, at least https://archives.miloush.net/michkap/archive/2005/01/16/353873.html suggests that the path case folding rules on Windows differed from unicode for some early versions and didn't change much later - I doubt if they have switched to be aligned with unicode.