NODE_PATH documentation appears to be incorrect

[evanw]

Context: evanw/esbuild#1117

I'm trying to replicate node's module resolution algorithm into esbuild, which is a JavaScript bundler that I'm building. I based my implementation on node's documented algorithm here: doc/api/modules.md. However, it seems like the documented algorithm is different than how node actually works. Specifically this part:

LOAD_NODE_MODULES(X, START)
1. let DIRS = NODE_MODULES_PATHS(START)
2. for each DIR in DIRS:
   a. LOAD_PACKAGE_EXPORTS(X, DIR)
   b. LOAD_AS_FILE(DIR/X)
   c. LOAD_AS_DIRECTORY(DIR/X)

NODE_MODULES_PATHS(START)
1. let PARTS = path split(START)
2. let I = count of PARTS - 1
3. let DIRS = [GLOBAL_FOLDERS]
4. while I >= 0,
   a. if PARTS[I] = "node_modules" CONTINUE
   b. DIR = path join(PARTS[0 .. I] + "node_modules")
   c. DIRS = DIRS + DIR
   d. let I = I - 1
5. return DIRS

This reads to me like the iteration order is for each DIR in [GLOBAL_FOLDERS] + ["node_modules" folders] so NODE_PATH takes precedence over all other folders. However, node actually behaves as if the iteration order is instead for each DIR in ["node_modules" folders] + [GLOBAL_FOLDERS]. Specifically this part in lib/internal/modules/cjs/loader.js):

  const nodePath = isWindows ? process.env.NODE_PATH : safeGetenv('NODE_PATH');
...
  const paths = [path.resolve(prefixDir, 'lib', 'node')];
...
  if (nodePath) {
    ArrayPrototypeUnshiftApply(paths, ArrayPrototypeFilter(
      StringPrototypeSplit(nodePath, path.delimiter),
      Boolean
    ));
  }

  modulePaths = paths;
...
    let paths = modulePaths;
    if (parent?.paths?.length) {
      paths = ArrayPrototypeConcat(parent.paths, paths);
    }

This is essentially _nodeModulePaths().concat(nodePath.split(':')) so all other folders take precedence over NODE_PATH.

Assuming the documentation is just incorrect, can the documentation be changed to reflect how node actually works? Or if not, is node's implementation of the algorithm incorrect and node itself should be fixed to match the algorithm?

[aduh95]

@nodejs/modules

[bmeck]

Yes, this should be changed in docs. You can see the "global directories" added in https://github.com/nodejs/node/blob/master/lib/internal/modules/cjs/loader.js#L677 . NOTE: global directories are only in CommonJS and are strictly not supported by ESM

[ljharb]

NODE_PATH is also widely discouraged in the ecosystem, and many tools don’t support it at all, including resolve. I’d suggest not adding it to esbuild.