Skip to content

Commit

Permalink
esm: skip file: URL conversion to path when possible
Browse files Browse the repository at this point in the history
PR-URL: nodejs/node#46305
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
sercher committed Apr 24, 2024
1 parent befe328 commit 7cfb6d8
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 8 deletions.
32 changes: 29 additions & 3 deletions graal-nodejs/lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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';
}
Expand All @@ -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);
Expand Down Expand Up @@ -128,4 +153,5 @@ module.exports = {
defaultGetFormat,
defaultGetFormatWithoutErrors,
extensionFormatMap,
extname,
};
7 changes: 2 additions & 5 deletions graal-nodejs/lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down Expand Up @@ -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);
Expand Down
27 changes: 27 additions & 0 deletions graal-nodejs/test/parallel/test-esm-url-extname.js
Original file line number Diff line number Diff line change
@@ -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));
});

0 comments on commit 7cfb6d8

Please sign in to comment.