diff --git a/CHANGELOG.md b/CHANGELOG.md index 5abcc836e2..0ea7700659 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa [#6562](https://github.com/yarnpkg/yarn/pull/6562) - [**Bertrand Marron**](https://github.com/tusbar) +- Fixes `require.resolve` when used together with the `paths` option + + [#6565](https://github.com/yarnpkg/yarn/pull/6565) - [**Maël Nison**](https://twitter.com/arcanis) + ## 1.12.0 - Adds initial support for PnP on Windows diff --git a/packages/pkg-tests/pkg-tests-specs/package.json b/packages/pkg-tests/pkg-tests-specs/package.json index 5ef9b6ab96..5b84ce264f 100644 --- a/packages/pkg-tests/pkg-tests-specs/package.json +++ b/packages/pkg-tests/pkg-tests-specs/package.json @@ -4,6 +4,7 @@ "main": "./sources/index.js", "dependencies": { "fs-extra": "^7.0.0", - "pkg-tests-core": "1.0.0" + "pkg-tests-core": "1.0.0", + "semver": "^5.6.0" } } diff --git a/packages/pkg-tests/pkg-tests-specs/sources/pnp.js b/packages/pkg-tests/pkg-tests-specs/sources/pnp.js index 3c70197991..ec95577320 100644 --- a/packages/pkg-tests/pkg-tests-specs/sources/pnp.js +++ b/packages/pkg-tests/pkg-tests-specs/sources/pnp.js @@ -1,6 +1,7 @@ const cp = require('child_process'); const {existsSync, statSync, stat, rename, readdir, remove} = require('fs-extra'); const {relative, isAbsolute} = require('path'); +const {satisfies} = require('semver'); const { fs: {createTemporaryFolder, readFile, readJson, writeFile, writeJson}, @@ -460,6 +461,110 @@ module.exports = makeTemporaryEnv => { ), ); + if (satisfies(process.versions.node, `>=8.9.0`)) { + test( + `it should support the 'paths' option from require.resolve (same dependency tree)`, + makeTemporaryEnv( + { + private: true, + workspaces: [`workspace-*`], + }, + { + plugNPlay: true, + }, + async ({path, run, source}) => { + await writeJson(`${path}/workspace-a/package.json`, { + name: `workspace-a`, + version: `1.0.0`, + dependencies: {[`no-deps`]: `1.0.0`}, + }); + + await writeJson(`${path}/workspace-b/package.json`, { + name: `workspace-b`, + version: `1.0.0`, + dependencies: {[`no-deps`]: `2.0.0`, [`one-fixed-dep`]: `1.0.0`}, + }); + + await run(`install`); + + await expect( + source( + `require(require.resolve('no-deps', {paths: ${JSON.stringify([ + `${path}/workspace-a`, + `${path}/workspace-b`, + ])}}))`, + ), + ).resolves.toMatchObject({ + name: `no-deps`, + version: `1.0.0`, + }); + }, + ), + ); + + // Skipped because not supported (we can't require files from within other dependency trees, since we couldn't + // reconcile them together: dependency tree A could think that package X has deps Y@1 while dependency tree B + // could think that X has deps Y@2 instead. Since they would share the same location on the disk, PnP wouldn't + // be able to tell which one should be used) + test.skip( + `it should support the 'paths' option from require.resolve (different dependency trees)`, + makeTemporaryEnv( + { + dependencies: {}, + }, + { + plugNPlay: true, + }, + async ({path, run, source}) => { + await run(`install`); + + const tmpA = await createTemporaryFolder(); + const tmpB = await createTemporaryFolder(); + + await writeJson(`${tmpA}/package.json`, { + dependencies: {[`no-deps`]: `1.0.0`}, + }); + + await writeJson(`${tmpB}/package.json`, { + dependencies: {[`no-deps`]: `2.0.0`, [`one-fixed-dep`]: `1.0.0`}, + }); + + await run(`install`, { + cwd: tmpA, + }); + + await run(`install`, { + cwd: tmpB, + }); + + await expect( + source(`require(require.resolve('no-deps', {paths: ${JSON.stringify([tmpA, tmpB])}}))`), + ).resolves.toMatchObject({ + name: `no-deps`, + version: `1.0.0`, + }); + }, + ), + ); + + test( + `using require.resolve with unsupported options should throw`, + makeTemporaryEnv( + { + dependencies: {[`no-deps`]: `1.0.0`}, + }, + { + plugNPlay: true, + }, + async ({path, run, source}) => { + await run(`install`); + + await expect(source(`require.resolve('no-deps', {foobar: 42})`)).rejects.toBeTruthy(); + }, + ), + ); + } + test( `it should load the index.js file when loading from a folder`, makeTemporaryEnv({}, {plugNPlay: true}, async ({path, run, source}) => { diff --git a/packages/pkg-tests/yarn.lock b/packages/pkg-tests/yarn.lock index 8adeb97797..c6cc754672 100644 --- a/packages/pkg-tests/yarn.lock +++ b/packages/pkg-tests/yarn.lock @@ -3150,6 +3150,11 @@ sax@^1.2.4: version "5.5.0" resolved "https://registry.yarnpkg.com/semver/-/semver-5.5.0.tgz#dc4bbc7a6ca9d916dee5d43516f0092b58f7b8ab" +semver@^5.6.0: + version "5.6.0" + resolved "https://registry.yarnpkg.com/semver/-/semver-5.6.0.tgz#7e74256fbaa49c75aa7c7a205cc22799cac80004" + integrity sha512-RS9R6R35NYgQn++fkDWaOmqGoj4Ek9gGs+DPxNUZKuwE183xjJroKvyo1IzVFeXvUrvmALy6FWD5xrdJT25gMg== + set-blocking@^2.0.0, set-blocking@~2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/set-blocking/-/set-blocking-2.0.0.tgz#045f9782d011ae9a6803ddd382b24392b3d890f7" diff --git a/src/util/generate-pnp-map-api.tpl.js b/src/util/generate-pnp-map-api.tpl.js index f594353b07..693b82394a 100644 --- a/src/util/generate-pnp-map-api.tpl.js +++ b/src/util/generate-pnp-map-api.tpl.js @@ -647,11 +647,47 @@ exports.setup = function setup() { return originalModuleResolveFilename.call(Module, request, parent, isMain, options); } - const issuerModule = getIssuerModule(parent); - const issuer = issuerModule ? issuerModule.filename : process.cwd() + '/'; + let issuers; - const resolution = exports.resolveRequest(request, issuer); - return resolution !== null ? resolution : request; + if (options) { + const optionNames = new Set(Object.keys(options)); + optionNames.delete('paths'); + + if (optionNames.size > 0) { + throw makeError( + `UNSUPPORTED`, + `Some options passed to require() aren't supported by PnP yet (${Array.from(optionNames).join(', ')})`, + ); + } + + if (options.paths) { + issuers = options.paths.map(entry => `${path.normalize(entry)}/`); + } + } + + if (!issuers) { + const issuerModule = getIssuerModule(parent); + const issuer = issuerModule ? issuerModule.filename : `${process.cwd()}/`; + + issuers = [issuer]; + } + + let firstError; + + for (const issuer of issuers) { + let resolution; + + try { + resolution = exports.resolveRequest(request, issuer); + } catch (error) { + firstError = firstError || error; + continue; + } + + return resolution !== null ? resolution : request; + } + + throw firstError; }; const originalFindPath = Module._findPath;