esm: skip file: URL conversion to path when possible

PR-URL: #46305 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Nov 10, 2023 · dfa4444 · dfa4444
1 parent b8634ee
commit dfa4444
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 8 deletions.
diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js
@@ -4,9 +4,10 @@ const {
   ObjectPrototypeHasOwnProperty,
   PromisePrototypeThen,
   PromiseResolve,
+  StringPrototypeCharCodeAt,
   StringPrototypeSlice,
 } = primordials;
-const { basename, extname, relative } = require('path');
+const { basename, relative } = require('path');
 const { getOptionValue } = require('internal/options');
 const {
   extensionFormatMap,
@@ -44,15 +45,38 @@ function getDataProtocolModuleFormat(parsed) {
   return mimeToFormat(mime);
 }
 
+const DOT_CODE = 46;
+const SLASH_CODE = 47;
+
+/**
+ * Returns the file extension from a URL. Should give similar result to
+ * `require('node:path').extname(require('node:url').fileURLToPath(url))`
+ * when used with a `file:` URL.
+ * @param {URL} url
+ * @returns {string}
+ */
+function extname(url) {
+  const { pathname } = url;
+  for (let i = pathname.length - 1; i > 0; i--) {
+    switch (StringPrototypeCharCodeAt(pathname, i)) {
+      case SLASH_CODE:
+        return '';
+
+      case DOT_CODE:
+        return StringPrototypeCharCodeAt(pathname, i - 1) === SLASH_CODE ? '' : StringPrototypeSlice(pathname, i);
+    }
+  }
+  return '';
+}
+
 /**
  * @param {URL} url
  * @param {{parentURL: string}} context
  * @param {boolean} ignoreErrors
  * @returns {string}
  */
 function getFileProtocolModuleFormat(url, context, ignoreErrors) {
-  const filepath = fileURLToPath(url);
-  const ext = extname(filepath);
+  const ext = extname(url);
   if (ext === '.js') {
     return getPackageType(url) === 'module' ? 'module' : 'commonjs';
   }
@@ -63,6 +87,7 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
   if (experimentalSpecifierResolution !== 'node') {
     // Explicit undefined return indicates load hook should rerun format check
     if (ignoreErrors) return undefined;
+    const filepath = fileURLToPath(url);
     let suggestion = '';
     if (getPackageType(url) === 'module' && ext === '') {
       const config = getPackageScopeConfig(url);
@@ -128,4 +153,5 @@ module.exports = {
   defaultGetFormat,
   defaultGetFormatWithoutErrors,
   extensionFormatMap,
+  extname,
 };
diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -35,8 +35,7 @@ const {
   Module: CJSModule,
   cjsParseCache,
 } = require('internal/modules/cjs/loader');
-const internalURLModule = require('internal/url');
-const { fileURLToPath, URL } = require('url');
+const { fileURLToPath, URL } = require('internal/url');
 let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
   debug = fn;
 });
@@ -147,9 +146,7 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
                                                             isMain) {
   debug(`Translating CJSModule ${url}`);
 
-  let filename = internalURLModule.fileURLToPath(new URL(url));
-  if (isWindows)
-    filename = StringPrototypeReplaceAll(filename, '/', '\\');
+  const filename = fileURLToPath(new URL(url));
 
   if (!cjsParse) await initCJSParse();
   const { module, exportNames } = cjsPreparseModuleExports(filename);

diff --git a/test/parallel/test-esm-url-extname.js b/test/parallel/test-esm-url-extname.js
@@ -0,0 +1,27 @@
+// Flags: --expose-internals
+'use strict';
+require('../common');
+const assert = require('node:assert');
+const path = require('node:path');
+const { extname } = require('node:internal/modules/esm/get_format');
+const { fileURLToPath } = require('node:url');
+
+[
+  'file:///c:/path/to/file',
+  'file:///c:/path/to/file.ext',
+  'file:///c:/path.to/file.ext',
+  'file:///c:/path.to/file',
+  'file:///c:/path.to/.file',
+  'file:///c:/path.to/.file.ext',
+  'file:///c:/path/to/f.ext',
+  'file:///c:/path/to/..ext',
+  'file:///c:/path/to/..',
+  'file:///c:/file',
+  'file:///c:/file.ext',
+  'file:///c:/.file',
+  'file:///c:/.file.ext',
+].forEach((input) => {
+  const inputAsURL = new URL(input);
+  const inputAsPath = fileURLToPath(inputAsURL);
+  assert.strictEqual(extname(inputAsURL), path.extname(inputAsPath));
+});