Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements require.resolve's paths options #6565

Merged
merged 4 commits into from Oct 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -36,6 +36,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
arcanis marked this conversation as resolved.
Show resolved Hide resolved
arcanis marked this conversation as resolved.
Show resolved Hide resolved

[#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
Expand Down
3 changes: 2 additions & 1 deletion packages/pkg-tests/pkg-tests-specs/package.json
Expand Up @@ -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"
}
}
105 changes: 105 additions & 0 deletions 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},
Expand Down Expand Up @@ -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}) => {
Expand Down
5 changes: 5 additions & 0 deletions packages/pkg-tests/yarn.lock
Expand Up @@ -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"
Expand Down
44 changes: 40 additions & 4 deletions src/util/generate-pnp-map-api.tpl.js
Expand Up @@ -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;
Expand Down