From 615b0ae307b371de67a8f9bec1546df4b0ecb087 Mon Sep 17 00:00:00 2001 From: per4uk Date: Wed, 10 Jan 2024 22:26:07 +0000 Subject: [PATCH] module: fix `--preserve-symlinks-main` Fixes resolving main module when the `argv[1]` was pointing to a symlink without its file extension. PR-URL: https://github.com/nodejs/node/pull/51312 Fixes: https://github.com/nodejs/node/issues/41000 Reviewed-By: Antoine du Hamel --- lib/internal/modules/cjs/loader.js | 6 +- .../test-esm-preserve-symlinks-main.js | 73 ++++++++++++------- 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 44abacb41a3430..e5b47d8874aeb7 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -469,15 +469,15 @@ function tryPackage(requestPath, exts, isMain, originalPath) { } /** - * Check if the file exists and is not a directory if using `--preserve-symlinks` and `isMain` is false, keep symlinks - * intact, otherwise resolve to the absolute realpath. + * Check if the file exists and is not a directory if using `--preserve-symlinks` and `isMain` is false or + * `--preserve-symlinks-main` and `isMain` is true , keep symlinks intact, otherwise resolve to the absolute realpath. * @param {string} requestPath The path to the file to load. * @param {boolean} isMain Whether the file is the main module. */ function tryFile(requestPath, isMain) { const rc = _stat(requestPath); if (rc !== 0) { return; } - if (getOptionValue('--preserve-symlinks') && !isMain) { + if (getOptionValue(isMain ? '--preserve-symlinks-main' : '--preserve-symlinks')) { return path.resolve(requestPath); } return toRealPath(requestPath); diff --git a/test/es-module/test-esm-preserve-symlinks-main.js b/test/es-module/test-esm-preserve-symlinks-main.js index 6f921f656fe22f..5eb58ca8c1bbbd 100644 --- a/test/es-module/test-esm-preserve-symlinks-main.js +++ b/test/es-module/test-esm-preserve-symlinks-main.js @@ -1,12 +1,19 @@ 'use strict'; -const common = require('../common'); -const { spawn } = require('child_process'); -const assert = require('assert'); -const path = require('path'); -const fs = require('fs'); - +const { spawnPromisified, skip } = require('../common'); const tmpdir = require('../common/tmpdir'); + +// Invoke the main file via a symlink. In this case --preserve-symlinks-main +// dictates that it'll resolve relative imports in the main file relative to +// the symlink, and not relative to the symlink target; the file structure set +// up below requires this to not crash when loading ./submodule_link.js + +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); +const { execPath } = require('node:process'); +const { describe, it } = require('node:test'); + tmpdir.refresh(); const tmpDir = tmpdir.path; @@ -14,7 +21,7 @@ fs.mkdirSync(path.join(tmpDir, 'nested')); fs.mkdirSync(path.join(tmpDir, 'nested2')); const entry = path.join(tmpDir, 'nested', 'entry.js'); -const entry_link_absolute_path = path.join(tmpDir, 'link.js'); +const entry_link_absolute_path = path.join(tmpDir, 'index.js'); const submodule = path.join(tmpDir, 'nested2', 'submodule.js'); const submodule_link_absolute_path = path.join(tmpDir, 'submodule_link.js'); @@ -31,27 +38,39 @@ try { fs.symlinkSync(submodule, submodule_link_absolute_path); } catch (err) { if (err.code !== 'EPERM') throw err; - common.skip('insufficient privileges for symlinks'); + skip('insufficient privileges for symlinks'); } -function doTest(flags, done) { - // Invoke the main file via a symlink. In this case --preserve-symlinks-main - // dictates that it'll resolve relative imports in the main file relative to - // the symlink, and not relative to the symlink target; the file structure set - // up above requires this to not crash when loading ./submodule_link.js - spawn(process.execPath, [ - '--preserve-symlinks', - '--preserve-symlinks-main', - entry_link_absolute_path, - ], { stdio: 'inherit' }) - .on('exit', (code) => { - assert.strictEqual(code, 0); - done(); - }); -} +describe('Invoke the main file via a symlink.', { concurrency: true }, () => { + it('should resolve relative imports in the main file', async () => { + const { code } = await spawnPromisified(execPath, [ + '--preserve-symlinks', + '--preserve-symlinks-main', + entry_link_absolute_path, + ]); + + assert.strictEqual(code, 0); + }); + + it('should resolve relative imports in the main file when file extension is omitted', async () => { + const entry_link_absolute_path_without_ext = path.join(tmpDir, 'index'); + + const { code } = await spawnPromisified(execPath, [ + '--preserve-symlinks', + '--preserve-symlinks-main', + entry_link_absolute_path_without_ext, + ]); + + assert.strictEqual(code, 0); + }); + + it('should resolve relative imports in the main file when filename(index.js) is omitted', async () => { + const { code } = await spawnPromisified(execPath, [ + '--preserve-symlinks', + '--preserve-symlinks-main', + tmpDir, + ]); -// First test the commonjs module loader -doTest([], () => { - // Now test the new loader - doTest([], () => {}); + assert.strictEqual(code, 0); + }); });