Skip to content

Commit

Permalink
Fixes invalid option forwarding (#6479)
Browse files Browse the repository at this point in the history
* Fixes typo

* Update CHANGELOG.md

* Adds tests

* Also forwards considerBuiltins to the symlinked path detection

* Uncomments the tests
  • Loading branch information
arcanis committed Oct 9, 2018
1 parent 743c145 commit 276720b
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 3 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Expand Up @@ -4,7 +4,11 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa

## Master

- Fixes handling of empty string entries for bin in package.json
- Fixes the `extensions` option when used by `resolveRequest`

[#6479](https://github.com/yarnpkg/yarn/pull/6479) - [**Maël Nison**](https://twitter.com/arcanis)

- Fixes handling of empty string entries for `bin` in package.json

[#6515](https://github.com/yarnpkg/yarn/pull/6515) - [**Ryan Burrows**](https://github.com/rhburrows)

Expand Down
1 change: 1 addition & 0 deletions packages/pkg-tests/pkg-tests-specs/sources/index.js
Expand Up @@ -4,5 +4,6 @@ exports.basic = require('./basic');
exports.dragon = require('./dragon');
exports.lock = require('./lock');
exports.pnp = require('./pnp');
exports.pnpapiV1 = require('./pnpapi-v1');
exports.script = require('./script');
exports.workspace = require('./workspace');
88 changes: 88 additions & 0 deletions packages/pkg-tests/pkg-tests-specs/sources/pnpapi-v1.js
@@ -0,0 +1,88 @@
const {fs: {writeFile, writeJson}} = require('pkg-tests-core');

module.exports = makeTemporaryEnv => {
describe(`Plug'n'Play API (v1)`, () => {
test(
`it should expost VERSIONS`,
makeTemporaryEnv({}, {plugNPlay: true}, async ({path, run, source}) => {
await run(`install`);

await expect(source(`require('pnpapi').VERSIONS`)).resolves.toMatchObject({std: 1});
}),
);

test(
`it should expost resolveToUnqualified`,
makeTemporaryEnv({}, {plugNPlay: true}, async ({path, run, source}) => {
await run(`install`);

await expect(source(`typeof require('pnpapi').resolveToUnqualified`)).resolves.toEqual(`function`);
}),
);

test(
`it should expost resolveToUnqualified`,
makeTemporaryEnv({}, {plugNPlay: true}, async ({path, run, source}) => {
await run(`install`);

await expect(source(`typeof require('pnpapi').resolveUnqualified`)).resolves.toEqual(`function`);
}),
);

test(
`it should expost resolveToUnqualified`,
makeTemporaryEnv({}, {plugNPlay: true}, async ({path, run, source}) => {
await run(`install`);

await expect(source(`typeof require('pnpapi').resolveRequest`)).resolves.toEqual(`function`);
}),
);

describe(`resolveRequest`, () => {
test(
`it should return null for builtins`,
makeTemporaryEnv({}, {plugNPlay: true}, async ({path, run, source}) => {
await run(`install`);

await expect(source(`require('pnpapi').resolveRequest('fs', '${path}/')`)).resolves.toEqual(null);
}),
);

test(
`it should support the 'considerBuiltins' option`,
makeTemporaryEnv(
{
dependencies: {[`fs`]: `link:./fs`},
},
{plugNPlay: true},
async ({path, run, source}) => {
await writeFile(`${path}/fs/index.js`, `module.exports = 'Hello world';`);
await writeJson(`${path}/fs/package.json`, {
name: `fs`,
version: `1.0.0`,
});

await run(`install`);

await expect(
source(`require('pnpapi').resolveRequest('fs', '${path}/', {considerBuiltins: false})`),
).resolves.toEqual(`${path}/fs/index.js`);
},
),
);

test(
`it should support the 'extensions' option`,
makeTemporaryEnv({}, {plugNPlay: true}, async ({path, run, source}) => {
await writeFile(`${path}/foo.bar`, `hello world`);

await run(`install`);

await expect(
source(`require('pnpapi').resolveRequest('./foo', '${path}/', {extensions: ['.bar']})`),
).resolves.toEqual(`${path}/foo.bar`);
}),
);
});
});
};
2 changes: 2 additions & 0 deletions packages/pkg-tests/yarn.test.js
Expand Up @@ -12,6 +12,7 @@ const {
dragon: dragonSpecs,
lock: lockSpecs,
pnp: pnpSpecs,
pnpapiV1: pnpapiV1Specs,
script: scriptSpecs,
workspace: workspaceSpecs,
} = require(`pkg-tests-specs`);
Expand Down Expand Up @@ -74,4 +75,5 @@ lockSpecs(pkgDriver);
scriptSpecs(pkgDriver);
workspaceSpecs(pkgDriver);
pnpSpecs(pkgDriver);
pnpapiV1Specs(pkgDriver);
dragonSpecs(pkgDriver);
4 changes: 2 additions & 2 deletions src/util/generate-pnp-map-api.tpl.js
Expand Up @@ -518,7 +518,7 @@ exports.resolveRequest = function resolveRequest(request, issuer, {considerBuilt
}

try {
exports.resolveToUnqualified(request, realIssuer, {extensions});
exports.resolveToUnqualified(request, realIssuer, {considerBuiltins});
} catch (error) {
// If an error was thrown, the problem doesn't seem to come from a path not being normalized, so we
// can just throw the original error which was legit.
Expand Down Expand Up @@ -547,7 +547,7 @@ exports.resolveRequest = function resolveRequest(request, issuer, {considerBuilt
}

try {
return exports.resolveUnqualified(unqualifiedPath);
return exports.resolveUnqualified(unqualifiedPath, {extensions});
} catch (resolutionError) {
if (resolutionError.code === 'QUALIFIED_PATH_RESOLUTION_FAILED') {
Object.assign(resolutionError.data, {request, issuer});
Expand Down

0 comments on commit 276720b

Please sign in to comment.