esm: cpp refactoring update not to use uv_fs_realpath

Author: guybedford

lib/internal/modules/esm/default_resolve.js

@@ -1,16 +1,27 @@
 'use strict';
 
+const internalFS = require('internal/fs/utils');
 const { NativeModule } = require('internal/bootstrap/loaders');
-const { ERR_TYPE_MISMATCH,
-        ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
+const { extname } = require('path');
+const { realpathSync } = require('fs');
 const { getOptionValue } = require('internal/options');
+
+const preserveSymlinks = getOptionValue('--preserve-symlinks');
+const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
 const experimentalJsonModules = getOptionValue('--experimental-json-modules');
-const { resolve: moduleWrapResolve } = internalBinding('module_wrap');
+
+const { resolve: moduleWrapResolve,
+        getPackageType } = internalBinding('module_wrap');
 const { pathToFileURL, fileURLToPath } = require('internal/url');
 const asyncESM = require('internal/process/esm_loader');
-const preserveSymlinks = getOptionValue('--preserve-symlinks');
-const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
-const { extname } = require('path');
+const { ERR_TYPE_MISMATCH,
+        ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
+
+const realpathCache = new Map();
+
+// const TYPE_NONE = 0;
+const TYPE_COMMONJS = 1;
+const TYPE_MODULE = 2;
 
 const extensionFormatMap = {
   '__proto__': null,
@@ -50,13 +61,24 @@ function resolve(specifier, parentURL) {
   if (isMain)
     parentURL = pathToFileURL(`${process.cwd()}/`).href;
 
-  const { url, type } =
-      moduleWrapResolve(specifier, parentURL,
-                        isMain ? !preserveSymlinksMain : !preserveSymlinks);
+  let url = moduleWrapResolve(specifier, parentURL);
+
+  if (isMain ? !preserveSymlinksMain : !preserveSymlinks) {
+    const real = realpathSync(fileURLToPath(url), {
+      [internalFS.realpathCacheKey]: realpathCache
+    });
+    const old = url;
+    url = pathToFileURL(real);
+    url.search = old.search;
+    url.hash = old.hash;
+  }
+
+  const type = getPackageType(url.href);
 
   const ext = extname(url.pathname);
-  let format =
-      (type !== 2 ? legacyExtensionFormatMap : extensionFormatMap)[ext];
+  const extMap =
+      type !== TYPE_MODULE ? legacyExtensionFormatMap : extensionFormatMap;
+  let format = extMap[ext];
 
   if (isMain && asyncESM.typeFlag) {
     // Conflict between explicit extension (.mjs, .cjs) and --type
@@ -68,8 +90,8 @@ function resolve(specifier, parentURL) {
 
     // Conflict between package scope type and --type
     if (ext === '.js') {
-      if (type === 2 && asyncESM.typeFlag === 'commonjs' ||
-          type === 1 && asyncESM.typeFlag === 'module') {
+      if (type === TYPE_MODULE && asyncESM.typeFlag === 'commonjs' ||
+          type === TYPE_COMMONJS && asyncESM.typeFlag === 'module') {
         throw new ERR_TYPE_MISMATCH(
           fileURLToPath(url), ext, asyncESM.typeFlag, 'scope');
       }
@@ -79,7 +101,7 @@ function resolve(specifier, parentURL) {
     if (isMain && asyncESM.typeFlag)
       format = asyncESM.typeFlag;
     else if (isMain)
-      format = type === 2 ? 'module' : 'commonjs';
+      format = type === TYPE_MODULE ? 'module' : 'commonjs';
     else
       throw new ERR_UNKNOWN_FILE_EXTENSION(fileURLToPath(url),
                                            fileURLToPath(parentURL));

src/env.h

@@ -86,7 +86,7 @@ struct PackageConfig {
     enum Bool { No, Yes };
   };
   struct PackageType {
-    enum Type : uint32_t { None, CommonJS, Module };
+    enum Type : uint32_t { None = 0, CommonJS, Module };
   };
   const Exists::Bool exists;
   const IsValid::Bool is_valid;

src/module_wrap.cc

@@ -476,17 +476,6 @@ std::string ReadFile(uv_file file) {
   return contents;
 }
 
-Maybe<std::string> RealPath(const std::string& path) {
-  uv_fs_t fs_req;
-  const int r = uv_fs_realpath(nullptr, &fs_req, path.c_str(), nullptr);
-  if (r < 0) {
-    return Nothing<std::string>();
-  }
-  std::string link_path = std::string(static_cast<const char*>(fs_req.ptr));
-  uv_fs_req_cleanup(&fs_req);
-  return Just(link_path);
-}
-
 enum DescriptorType {
   FILE,
   DIRECTORY,
@@ -895,21 +884,11 @@ Maybe<URL> Resolve(Environment* env,
   return FinalizeResolution(env, resolved, base);
 }
 
-Maybe<PackageType::Type> GetPackageType(Environment* env,
-                                        const URL& resolved) {
-  Maybe<const PackageConfig*> pcfg =
-      GetPackageScopeConfig(env, resolved, resolved);
-  if (pcfg.IsNothing()) {
-    return Nothing<PackageType::Type>();
-  }
-  return Just(pcfg.FromJust()->type);
-}
-
 void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  // module.resolve(specifier, url, realpath)
-  CHECK_EQ(args.Length(), 3);
+  // module.resolve(specifier, url)
+  CHECK_EQ(args.Length(), 2);
 
   CHECK(args[0]->IsString());
   Utf8Value specifier_utf8(env->isolate(), args[0]);
@@ -919,9 +898,6 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& args) {
   Utf8Value url_utf8(env->isolate(), args[1]);
   URL url(*url_utf8, url_utf8.length());
 
-  CHECK(args[2]->IsBoolean());
-  bool realpath = args[2]->IsTrue();
-
   if (url.flags() & URL_FLAGS_FAILED) {
     return node::THROW_ERR_INVALID_ARG_TYPE(
         env, "second argument is not a URL string");
@@ -942,47 +918,27 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& args) {
   URL resolution = result.FromJust();
   CHECK(!(resolution.flags() & URL_FLAGS_FAILED));
 
-  if (realpath) {
-    Maybe<std::string> real_resolution =
-        node::loader::RealPath(resolution.ToFilePath());
-    // Note: Ensure module resolve FS error handling consistency
-    if (real_resolution.IsNothing()) {
-      std::string msg = "realpath error resolving " + resolution.ToFilePath();
-      env->ThrowError(msg.c_str());
-      try_catch.ReThrow();
-      return;
-    }
-    std::string fragment = resolution.fragment();
-    std::string query = resolution.query();
-    resolution = URL::FromFilePath(real_resolution.FromJust());
-    resolution.set_fragment(fragment);
-    resolution.set_query(query);
-  }
+  args.GetReturnValue().Set(resolution.ToObject(env).ToLocalChecked());
+}
 
-  Maybe<PackageType::Type> pkg_type =
-      node::loader::GetPackageType(env, resolution);
-  if (result.IsNothing()) {
-    CHECK(try_catch.HasCaught());
-    try_catch.ReThrow();
-    return;
-  }
-  CHECK(!try_catch.HasCaught());
+void ModuleWrap::GetPackageType(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
 
-  Local<Object> resolved = Object::New(env->isolate());
+  // module.getPackageType(url)
+  CHECK_EQ(args.Length(), 1);
 
-  resolved->DefineOwnProperty(
-    env->context(),
-    env->type_string(),
-    Integer::New(env->isolate(), pkg_type.FromJust()),
-    v8::ReadOnly).FromJust();
+  CHECK(args[0]->IsString());
+  Utf8Value url_utf8(env->isolate(), args[0]);
+  URL url(*url_utf8, url_utf8.length());
 
-  resolved->DefineOwnProperty(
-    env->context(),
-    env->url_string(),
-    resolution.ToObject(env).ToLocalChecked(),
-    v8::ReadOnly).FromJust();
+  PackageType::Type pkg_type = PackageType::None;
+  Maybe<const PackageConfig*> pcfg =
+      GetPackageScopeConfig(env, url, url);
+  if (!pcfg.IsNothing()) {
+    pkg_type = pcfg.FromJust()->type;
+  }
 
-  args.GetReturnValue().Set(resolved);
+  args.GetReturnValue().Set(Integer::New(env->isolate(), pkg_type));
 }
 
 static MaybeLocal<Promise> ImportModuleDynamically(
@@ -1118,6 +1074,7 @@ void ModuleWrap::Initialize(Local<Object> target,
   target->Set(env->context(), FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"),
               tpl->GetFunction(context).ToLocalChecked()).FromJust();
   env->SetMethod(target, "resolve", Resolve);
+  env->SetMethod(target, "getPackageType", GetPackageType);
   env->SetMethod(target,
                  "setImportModuleDynamicallyCallback",
                  SetImportModuleDynamicallyCallback);

src/module_wrap.h

@@ -65,7 +65,7 @@ class ModuleWrap : public BaseObject {
       const v8::FunctionCallbackInfo<v8::Value>& args);
 
   static void Resolve(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void SetScopeType(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void GetPackageType(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetImportModuleDynamicallyCallback(
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetInitializeImportMetaObjectCallback(

src/node_url.h

@@ -153,14 +153,6 @@ class URL {
     return context_.fragment;
   }
 
-  void set_fragment(const std::string& fragment) {
-    context_.fragment = fragment;
-  }
-
-  void set_query(const std::string& query) {
-    context_.query = query;
-  }
-
   std::string path() const {
     std::string ret;
     for (const std::string& element : context_.path) {

test/es-module/test-esm-symlink-type.js

@@ -64,7 +64,7 @@ symlinks.forEach((symlink) => {
                     stdout.includes(symlink.prints)) return;
           assert.fail(`For ${JSON.stringify(symlink)}, ${
             (symlink.errorsWithPreserveSymlinksMain) ?
-              'failed to error' : `errored unexpectedly\n${err}`
+              'failed to error' : 'errored unexpectedly'
           } with --preserve-symlinks-main`);
         } else {
           if (stdout.includes(symlink.prints)) return;