Skip to content

Commit

Permalink
Merge pull request #43 from arcanis/fix-npm-run
Browse files Browse the repository at this point in the history
Prevents pnp from bootstrapping when running non-pnp-installed scripts
  • Loading branch information
arcanis authored Mar 27, 2018
2 parents 7bf140b + ff45d9a commit 66b2252
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 16 deletions.
22 changes: 21 additions & 1 deletion packages/pkg-tests/pkg-tests-specs/sources/pnp.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const {fs: {writeFile, writeJson}, tests: {getPackageDirectoryPath}} = require('pkg-tests-core');
const {fs: {createTemporaryFolder, writeFile, writeJson}, tests: {getPackageDirectoryPath}} = require('pkg-tests-core');

module.exports = makeTemporaryEnv => {
const {basic: basicSpecs, script: scriptSpecs, workspace: workspaceSpecs} = require('pkg-tests-specs');
Expand Down Expand Up @@ -335,4 +335,24 @@ module.exports = makeTemporaryEnv => {
),
);
});

test(
`it should not setup pnp when calling a script outside of the install tree`,
makeTemporaryEnv(
{},
{
plugNPlay: true,
},
async ({path, run, source}) => {
await run(`install`);

const tmp = await createTemporaryFolder();

await writeFile(`${tmp}/node_modules/dep/index.js`, `module.exports = 42;`);
await writeFile(`${tmp}/index.js`, `require('dep')`);

await run(`node`, `${tmp}/index.js`);
},
),
);
};
4 changes: 2 additions & 2 deletions packages/pkg-tests/pkg-tests-specs/sources/workspace.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
dependencies: {
[`workspace-a`]: `1.0.0`,
[`workspace-b`]: `1.0.0`,
}
},
},
async ({path, run, source}) => {
await writeJson(`${path}/packages/workspace-a/package.json`, {
Expand Down Expand Up @@ -96,7 +96,7 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
workspaces: [`packages/*`],
dependencies: {
[`workspace`]: `1.0.0`,
}
},
},
async ({path, run, source}) => {
await writeJson(`${path}/packages/workspace/package.json`, {
Expand Down
37 changes: 24 additions & 13 deletions src/util/generate-pnp-map-api.tpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ const moduleCache = new Map();

$$SETUP_STATIC_TABLES();

/**
* Returns the module that should be used to resolve require calls. It's usually the direct parent, except if we're
* inside an eval expression.
*/

function getIssuerModule(parent) {
let issuer = parent;

while (issuer && (issuer.id === '[eval]' || issuer.id === '<repl>' || !issuer.filename)) {
issuer = issuer.parent;
}

return issuer;
}

/**
* Returns information about a package in a safe way (will throw if they cannot be retrieved)
*/
Expand Down Expand Up @@ -244,16 +259,6 @@ exports.resolveRequest = function resolveRequest(request, issuer) {
*/

exports.setup = function setup() {
function getIssuer(parent) {
let issuer = parent;

while (issuer && (issuer.id === '[eval]' || issuer.id === '<repl>' || !issuer.filename)) {
issuer = issuer.parent;
}

return issuer;
}

Module._load = function(request, parent, isMain) {
// Builtins are managed by the regular Node loader

Expand Down Expand Up @@ -310,7 +315,7 @@ exports.setup = function setup() {
};

Module._resolveFilename = function(request, parent, isMain, options) {
const issuerModule = getIssuer(parent);
const issuerModule = getIssuerModule(parent);
const issuer = issuerModule ? issuerModule.filename : process.cwd() + path.sep;

return exports.resolveRequest(request, issuer);
Expand Down Expand Up @@ -388,6 +393,12 @@ exports.setupCompatibilityLayer = () => {
};

if (module.parent && module.parent.id === 'internal/preload') {
exports.setup();
exports.setupCompatibilityLayer();
const issuerPath = process.argv[1] || process.cwd() + path.sep;
const issuerLocator = exports.findPackageLocator(issuerPath);

// We don't want to boot pnp if the script being run isn't part of the project we've installed
if (issuerLocator) {
exports.setup();
exports.setupCompatibilityLayer();
}
}

0 comments on commit 66b2252

Please sign in to comment.