Skip to content

Commit

Permalink
module: fix --preserve-symlinks-main
Browse files Browse the repository at this point in the history
Fixes resolving main module when the `argv[1]` was pointing to a symlink
without its file extension.

PR-URL: nodejs#51312
Fixes: nodejs#41000
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
per4uk authored and Medhansh404 committed Jan 19, 2024
1 parent 3004ba1 commit 66f54cd
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 30 deletions.
6 changes: 3 additions & 3 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
73 changes: 46 additions & 27 deletions test/es-module/test-esm-preserve-symlinks-main.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
'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;

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');

Expand All @@ -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);
});
});

0 comments on commit 66f54cd

Please sign in to comment.