From ec2ffd6b9d255e19818b6949d2f7dc7ac70faee9 Mon Sep 17 00:00:00 2001 From: Daniele Belardi Date: Thu, 16 Jul 2020 09:27:53 +0200 Subject: [PATCH] module: self referential modules in repl or `-r` Load self referential modules from the repl and using the preload flag `-r`. In both cases the base path used for resolution is the current `process.cwd()`. Also fixes an internal cycle bug in the REPL exports resolution. PR-URL: https://github.com/nodejs/node/pull/32261 Fixes: https://github.com/nodejs/node/issues/31595 Reviewed-By: Guy Bedford Reviewed-By: Jan Krems --- lib/internal/modules/cjs/loader.js | 34 +++++++++++++++---- lib/internal/modules/esm/loader.js | 3 ++ lib/internal/modules/esm/resolve.js | 3 +- test/fixtures/self_ref_module/index.js | 4 +++ test/fixtures/self_ref_module/package.json | 13 +++++++ .../parallel/test-preload-self-referential.js | 20 +++++++++++ .../test-repl-require-self-referential.js | 25 ++++++++++++++ 7 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 test/fixtures/self_ref_module/index.js create mode 100644 test/fixtures/self_ref_module/package.json create mode 100644 test/parallel/test-preload-self-referential.js create mode 100644 test/parallel/test-repl-require-self-referential.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index eba9a54641b..de24f6c409c 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -29,6 +29,7 @@ module.exports = { const { ArrayIsArray, + ArrayPrototypeJoin, Error, JSONParse, Map, @@ -422,7 +423,23 @@ function findLongestRegisteredExtension(filename) { return '.js'; } +function trySelfParentPath(parent) { + if (!parent) return false; + + if (parent.filename) { + return parent.filename; + } else if (parent.id === '' || parent.id === 'internal/preload') { + try { + return process.cwd() + path.sep; + } catch { + return false; + } + } +} + function trySelf(parentPath, request) { + if (!parentPath) return false; + const { data: pkg, path: basePath } = readPackageScope(parentPath) || {}; if (!pkg || pkg.exports === undefined) return false; if (typeof pkg.name !== 'string') return false; @@ -1053,13 +1070,16 @@ Module._resolveFilename = function(request, parent, isMain, options) { } } } - const filename = trySelf(parent.filename, request); - if (filename) { - const cacheKey = request + '\x00' + - (paths.length === 1 ? paths[0] : paths.join('\x00')); - Module._pathCache[cacheKey] = filename; - return filename; - } + } + + // Try module self resoultion first + const parentPath = trySelfParentPath(parent); + const selfResolved = trySelf(parentPath, request); + if (selfResolved) { + const cacheKey = request + '\x00' + + (paths.length === 1 ? paths[0] : ArrayPrototypeJoin(paths, '\x00')); + Module._pathCache[cacheKey] = selfResolved; + return selfResolved; } // Look up the filename first, since that's the cache key. diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index de08845a820..37191f65bf0 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -1,5 +1,8 @@ 'use strict'; +// This is needed to avoid cycles in esm/resolve <-> cjs/loader +require('internal/modules/cjs/loader'); + const { FunctionPrototypeBind, ObjectSetPrototypeOf, diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 16ca78880c7..7ea59f30c68 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -32,7 +32,6 @@ const { } = require('fs'); const { getOptionValue } = require('internal/options'); const { sep, relative } = require('path'); -const { Module: CJSModule } = require('internal/modules/cjs/loader'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); const typeFlag = getOptionValue('--input-type'); @@ -49,11 +48,13 @@ const { ERR_UNSUPPORTED_DIR_IMPORT, ERR_UNSUPPORTED_ESM_URL_SCHEME, } = require('internal/errors').codes; +const { Module: CJSModule } = require('internal/modules/cjs/loader'); const packageJsonReader = require('internal/modules/package_json_reader'); const DEFAULT_CONDITIONS = ObjectFreeze(['node', 'import']); const DEFAULT_CONDITIONS_SET = new SafeSet(DEFAULT_CONDITIONS); + function getConditionsSet(conditions) { if (conditions !== undefined && conditions !== DEFAULT_CONDITIONS) { if (!ArrayIsArray(conditions)) { diff --git a/test/fixtures/self_ref_module/index.js b/test/fixtures/self_ref_module/index.js new file mode 100644 index 00000000000..7faa73693b5 --- /dev/null +++ b/test/fixtures/self_ref_module/index.js @@ -0,0 +1,4 @@ +'use strict' + +module.exports = 'Self resolution working'; + diff --git a/test/fixtures/self_ref_module/package.json b/test/fixtures/self_ref_module/package.json new file mode 100644 index 00000000000..7280b184c71 --- /dev/null +++ b/test/fixtures/self_ref_module/package.json @@ -0,0 +1,13 @@ +{ + "name": "self_ref", + "version": "1.0.0", + "description": "", + "main": "index.js", + "exports": "./index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "keywords": [], + "author": "", + "license": "ISC" +} diff --git a/test/parallel/test-preload-self-referential.js b/test/parallel/test-preload-self-referential.js new file mode 100644 index 00000000000..2624527deb3 --- /dev/null +++ b/test/parallel/test-preload-self-referential.js @@ -0,0 +1,20 @@ +'use strict'; + +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const { exec } = require('child_process'); + +const nodeBinary = process.argv[0]; + +if (!common.isMainThread) + common.skip('process.chdir is not available in Workers'); + +const selfRefModule = fixtures.path('self_ref_module'); +const fixtureA = fixtures.path('printA.js'); + +exec(`"${nodeBinary}" -r self_ref "${fixtureA}"`, { cwd: selfRefModule }, + (err, stdout, stderr) => { + assert.ifError(err); + assert.strictEqual(stdout, 'A\n'); + }); diff --git a/test/parallel/test-repl-require-self-referential.js b/test/parallel/test-repl-require-self-referential.js new file mode 100644 index 00000000000..7ced6dbf117 --- /dev/null +++ b/test/parallel/test-repl-require-self-referential.js @@ -0,0 +1,25 @@ +'use strict'; + +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const { spawn } = require('child_process'); + +if (!common.isMainThread) + common.skip('process.chdir is not available in Workers'); + +const selfRefModule = fixtures.path('self_ref_module'); +const child = spawn(process.execPath, + ['--interactive'], + { cwd: selfRefModule } +); +let output = ''; +child.stdout.on('data', (chunk) => output += chunk); +child.on('exit', common.mustCall(() => { + const results = output.replace(/^> /mg, '').split('\n').slice(2); + assert.deepStrictEqual(results, [ "'Self resolution working'", '' ]); +})); + +child.stdin.write('require("self_ref");\n'); +child.stdin.write('.exit'); +child.stdin.end();