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

Automatically install missing dependencies, part 2 #805

Merged
merged 39 commits into from
Mar 18, 2018
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
866e2b7
Automagically npm install missing dependencies
davidnagli Feb 14, 2018
60e160d
Changed automatic install to use installPackage helper
davidnagli Feb 14, 2018
1cacb59
Merge branch 'master' into master
davidnagli Feb 14, 2018
bcbb07d
Added back yarn.lock check, improved yarn.lock & project root resolution
davidnagli Feb 15, 2018
0e80fe3
Merge remote-tracking branch 'origin/master'
davidnagli Feb 15, 2018
184e6f9
Bug fix
davidnagli Feb 16, 2018
1d30d0c
refactor/cleanup code to fix the issues @brandon93s pointed out
davidnagli Feb 16, 2018
8b98fee
Dev mode check
davidnagli Feb 16, 2018
9f27b5d
Added autoinstall cli flags
davidnagli Feb 17, 2018
2bd5410
Merge remote-tracking branch 'upstream/master'
davidnagli Feb 17, 2018
9308335
Fixed Node 6 Compatiblity Issues
davidnagli Feb 18, 2018
8354b61
Fixed refactoring mistake
davidnagli Feb 21, 2018
b3ba099
Removed recursive config file search
davidnagli Feb 21, 2018
b076b7d
Merge branch 'master' of https://github.com/davidnagli/parcel into da…
devongovett Feb 22, 2018
8a3dc51
autoinstall improvements
devongovett Feb 23, 2018
9680030
Merge branch 'master' of github.com:parcel-bundler/parcel into davidn…
devongovett Mar 6, 2018
6f4a194
Merge branch 'davidnagli-master' into devon-autoinstall
devongovett Mar 6, 2018
949c1df
Removed redundent variable projectRootLocation
davidnagli Mar 6, 2018
b3ba28e
Added back reccursive yarn.lock resolution
davidnagli Mar 6, 2018
63c34f5
CHANGED CONFIG.RESOLVE() TO CHECK THE DIRECORY BEFORE GOING TO PARENT
davidnagli Mar 6, 2018
b26cb28
Changed localrequire to pass in 'basedir' instead of 'path' to install()
davidnagli Mar 6, 2018
11a5c35
Added initial unit tests
davidnagli Mar 6, 2018
9c91de6
Whoops... removed test.only() filter from autoinstall unit tests
davidnagli Mar 6, 2018
9e06479
Merge branch 'master' of https://github.com/davidnagli/parcel
davidnagli Mar 6, 2018
bdb6485
Added support for absolute and tilde paths
davidnagli Mar 6, 2018
2356c2a
Refactored parameters into config object
davidnagli Mar 6, 2018
4cf5d2d
Fixed refactored usages, cleaned up unit tests
davidnagli Mar 7, 2018
06eecdd
Renamed 'config' to 'configObj'
davidnagli Mar 7, 2018
40efbb3
Merge branch 'master' of https://github.com/davidnagli/parcel into de…
devongovett Mar 7, 2018
89a668d
Cleanup / Refactoring - Split code into seperate functions
davidnagli Mar 8, 2018
674d458
Cleaned up unit tests
davidnagli Mar 8, 2018
2030211
Retrigger CI
davidnagli Mar 8, 2018
eb9c263
Merge branch 'master' into master
davidnagli Mar 13, 2018
b32e731
Merge branch 'master' of https://github.com/davidnagli/parcel into de…
devongovett Mar 17, 2018
37c412c
Merge branch 'master' of github.com:parcel-bundler/parcel into devon-…
devongovett Mar 18, 2018
3343f28
Autoinstall improvements
devongovett Mar 18, 2018
6e07e6d
Fix tests
devongovett Mar 18, 2018
c462a4e
Remove explicit package-manager argument for now
devongovett Mar 18, 2018
d2716cd
Fix line counting on windows
devongovett Mar 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 48 additions & 14 deletions src/Bundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const config = require('./utils/config');
const emoji = require('./utils/emoji');
const loadEnv = require('./utils/env');
const PromiseQueue = require('./utils/PromiseQueue');
const installPackage = require('./utils/installPackage');
const bundleReport = require('./utils/bundleReport');
const prettifyTime = require('./utils/prettifyTime');

Expand Down Expand Up @@ -96,7 +97,8 @@ class Bundler extends EventEmitter {
hmrHostname:
options.hmrHostname ||
(options.target === 'electron' ? 'localhost' : ''),
detailedReport: options.detailedReport || false
detailedReport: options.detailedReport || false,
autoinstall: (options.autoinstall || false) && !isProduction
};
}

Expand Down Expand Up @@ -325,38 +327,70 @@ class Bundler extends EventEmitter {
}
}

async resolveDep(asset, dep) {
async resolveDep(asset, dep, install = true) {
try {
return await this.resolveAsset(dep.name, asset.name);
} catch (err) {
let thrown = err;

if (thrown.message.indexOf(`Cannot find module '${dep.name}'`) === 0) {
// Check if dependency is a local file
let isLocalFile = /^[/~.]/.test(dep.name);
let fromNodeModules = asset.name.includes(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check here to make sure we don't auto install dependencies of things inside node_modules, only application code. If a dependency is missing from within a node_modules folder, that is a bug in the package and so we should error.

`${Path.sep}node_modules${Path.sep}`
);

// If it's not a local file, attempt to install the dep
if (
!isLocalFile &&
!fromNodeModules &&
this.options.autoinstall &&
install
) {
return await this.installDep(asset, dep);
}

// If the dep is optional, return before we throw
if (dep.optional) {
return;
}

thrown.message = `Cannot resolve dependency '${dep.name}'`;

// Add absolute path to the error message if the dependency specifies a relative path
if (dep.name.startsWith('.')) {
if (isLocalFile) {
const absPath = Path.resolve(Path.dirname(asset.name), dep.name);
err.message += ` at '${absPath}'`;
}

// Generate a code frame where the dependency was used
if (dep.loc) {
await asset.loadIfNeeded();
thrown.loc = dep.loc;
thrown = asset.generateErrorMessage(thrown);
thrown.message += ` at '${absPath}'`;
}

thrown.fileName = asset.name;
await this.throwDepError(asset, dep, thrown);
}

throw thrown;
}
}

async installDep(asset, dep) {
let [moduleName] = this.resolver.getModuleParts(dep.name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used this function to get the module name to install. If you required e.g. lodash/cloneDeep in your app, we want to install lodash, not the entire dep name. The exception is scoped packages, e.g. @scope/module. In that case, we pass the whole thing. the getModuleParts function from the resolver handles both of these cases.

try {
await installPackage([moduleName], asset.name, {saveDev: false});
} catch (err) {
await this.throwDepError(asset, dep, err);
}

return await this.resolveDep(asset, dep, false);
}

async throwDepError(asset, dep, err) {
// Generate a code frame where the dependency was used
if (dep.loc) {
await asset.loadIfNeeded();
err.loc = dep.loc;
err = asset.generateErrorMessage(err);
}

err.fileName = asset.name;
throw err;
}

async processAsset(asset, isRebuild) {
if (isRebuild) {
asset.invalidate();
Expand Down
11 changes: 10 additions & 1 deletion src/Logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,18 @@ class Logger {
this.chalk = new chalk.constructor({enabled: this.color});
}

countLines(message) {
return message.split('\n').reduce((p, line) => p + Math.ceil((line.length || 1) / process.stdout.columns), 0);
}

writeRaw(message) {
this.lines += this.countLines(message) - 1;
process.stdout.write(message);
}

write(message, persistent = false) {
if (!persistent) {
this.lines += message.split('\n').length;
this.lines += this.countLines(message);
}

this._log(message);
Expand Down
2 changes: 2 additions & 0 deletions src/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ program
.option('--no-hmr', 'disable hot module replacement')
.option('--no-cache', 'disable the filesystem cache')
.option('--no-source-maps', 'disable sourcemaps')
.option('--no-autoinstall', 'disable autoinstall')
.option(
'-t, --target [target]',
'set the runtime environment, either "node", "browser" or "electron". defaults to "browser"',
Expand Down Expand Up @@ -74,6 +75,7 @@ program
.option('--no-hmr', 'disable hot module replacement')
.option('--no-cache', 'disable the filesystem cache')
.option('--no-source-maps', 'disable sourcemaps')
.option('--no-autoinstall', 'disable autoinstall')
.option(
'-t, --target [target]',
'set the runtime environment, either "node", "browser" or "electron". defaults to "browser"',
Expand Down
24 changes: 19 additions & 5 deletions src/utils/PromiseQueue.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
class PromiseQueue {
constructor(callback) {
constructor(callback, options = {}) {
this.process = callback;
this.maxConcurrent = options.maxConcurrent || Infinity;
this.retry = options.retry !== false;
this.queue = [];
this.processing = new Set();
this.processed = new Set();
this.numRunning = 0;
this.runPromise = null;
this.resolve = null;
this.reject = null;
Expand All @@ -14,7 +17,7 @@ class PromiseQueue {
return;
}

if (this.runPromise) {
if (this.runPromise && this.numRunning < this.maxConcurrent) {
this._runJob(job, args);
} else {
this.queue.push([job, args]);
Expand All @@ -41,13 +44,24 @@ class PromiseQueue {

async _runJob(job, args) {
try {
this.numRunning++;
await this.process(job, ...args);
this.processing.delete(job);
this.processed.add(job);
this.numRunning--;
this._next();
} catch (err) {
this.queue.push([job, args]);
this.reject(err);
this.numRunning--;
if (this.retry) {
this.queue.push([job, args]);
} else {
this.processing.delete(job);
}

if (this.reject) {
this.reject(err);
}

this._reset();
}
}
Expand All @@ -58,7 +72,7 @@ class PromiseQueue {
}

if (this.queue.length > 0) {
while (this.queue.length > 0) {
while (this.queue.length > 0 && this.numRunning < this.maxConcurrent) {
this._runJob(...this.queue.shift());
}
} else if (this.processing.size === 0) {
Expand Down
134 changes: 87 additions & 47 deletions src/utils/installPackage.js
Original file line number Diff line number Diff line change
@@ -1,54 +1,55 @@
const spawn = require('cross-spawn');
const config = require('./config');
const path = require('path');
const promisify = require('./promisify');
const resolve = promisify(require('resolve'));
const commandExists = require('command-exists');
const logger = require('../Logger');
const emoji = require('./emoji');
const pipeSpawn = require('./pipeSpawn');
const PromiseQueue = require('./PromiseQueue');
const path = require('path');
const fs = require('./fs');

async function install(dir, modules, installPeers = true) {
let location = await config.resolve(dir, ['yarn.lock', 'package.json']);

return new Promise((resolve, reject) => {
let install;
let options = {
cwd: location ? path.dirname(location) : dir
};

if (location && path.basename(location) === 'yarn.lock') {
install = spawn('yarn', ['add', ...modules, '--dev'], options);
} else {
install = spawn('npm', ['install', ...modules, '--save-dev'], options);
}

install.stdout.pipe(process.stdout);
install.stderr.pipe(process.stderr);

install.on('close', async code => {
if (code !== 0) {
return reject(new Error(`Failed to install ${modules.join(', ')}.`));
}

if (!installPeers) {
return resolve();
}

try {
await Promise.all(modules.map(m => installPeerDependencies(dir, m)));
} catch (err) {
return reject(
new Error(
`Failed to install peerDependencies for ${modules.join(', ')}.`
)
);
}

resolve();
});
});
}
async function install(modules, filepath, options = {}) {
let {installPeers = true, saveDev = true, packageManager} = options;

logger.status(emoji.progress, `Installing ${modules.join(', ')}...`);

let packageLocation = await config.resolve(filepath, ['package.json']);
let cwd = packageLocation ? path.dirname(packageLocation) : process.cwd();

if (!packageManager) {
packageManager = await determinePackageManager(filepath);
}

let commandToUse = packageManager === 'npm' ? 'install' : 'add';
let args = [commandToUse, ...modules];
if (saveDev) {
args.push('-D');
} else if (packageManager === 'npm') {
args.push('--save');
}

// npm doesn't auto-create a package.json when installing,
// so create an empty one if needed.
if (packageManager === 'npm' && !packageLocation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this to make sure we create a package.json when needed.

await fs.writeFile(path.join(cwd, 'package.json'), '{}');
}

try {
await pipeSpawn(packageManager, args, {cwd});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to using the pipeSpawn utility for this.

} catch (err) {
throw new Error(`Failed to install ${modules.join(', ')}.`);
}

async function installPeerDependencies(dir, name) {
let basedir = path.dirname(dir);
if (installPeers) {
await Promise.all(
modules.map(m => installPeerDependencies(filepath, m, options))
);
}
}

async function installPeerDependencies(filepath, name, options) {
let basedir = path.dirname(filepath);
const [resolved] = await resolve(name, {basedir});
const pkg = await config.load(resolved, ['package.json']);
const peers = pkg.peerDependencies || {};
Expand All @@ -59,8 +60,47 @@ async function installPeerDependencies(dir, name) {
}

if (modules.length) {
await install(dir, modules, false);
await install(
modules,
filepath,
Object.assign({}, options, {installPeers: false})
);
}
}

module.exports = install;
async function determinePackageManager(filepath) {
let configFile = await config.resolve(filepath, [
'yarn.lock',
'package-lock.json'
]);
let hasYarn = await checkForYarnCommand();

// If Yarn isn't available, or there is a package-lock.json file, use npm.
let configName = configFile && path.basename(configFile);
if (!hasYarn || configName === 'package-lock.json') {
return 'npm';
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this logic slightly. Now we default to yarn if it is installed, but use npm if a package-lock.json file is found and yarn.lock is not, or yarn is not installed.


return 'yarn';
}

let hasYarn = null;
async function checkForYarnCommand() {
if (hasYarn != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caching if we have yarn or not

return hasYarn;
}

try {
hasYarn = await commandExists('yarn');
} catch (err) {
hasYarn = false;
}

return hasYarn;
}

let queue = new PromiseQueue(install, {maxConcurrent: 1, retry: false});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a queue for install calls so that we can never have more than one install happening at the same time. Otherwise, npm will fail trying to write over the same files simultaneously.

module.exports = function(...args) {
queue.add(...args);
return queue.run();
};
2 changes: 1 addition & 1 deletion src/utils/localRequire.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ async function localRequire(name, path, triedInstall = false) {
resolved = resolve.sync(name, {basedir});
} catch (e) {
if (e.code === 'MODULE_NOT_FOUND' && !triedInstall) {
await install(path, [name]);
await install([name], path);
return localRequire(name, path, true);
}
throw e;
Expand Down
16 changes: 13 additions & 3 deletions src/utils/pipeSpawn.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
const spawn = require('cross-spawn');
const logger = require('../Logger');

function pipeSpawn(cmd, params, opts) {
const cp = spawn(cmd, params, opts);
cp.stdout.pipe(process.stdout);
cp.stderr.pipe(process.stderr);
const cp = spawn(cmd, params, Object.assign({
env: Object.assign({
FORCE_COLOR: logger.color,
npm_config_color: logger.color ? 'always': '',
npm_config_progress: true
}, process.env)
}, opts));

cp.stdout.setEncoding('utf8').on('data', d => logger.writeRaw(d));
cp.stderr.setEncoding('utf8').on('data', d => logger.writeRaw(d));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pipeSpawn now uses the logger to write its output rather than directly piping to stdout. This creates a better experience when installing packages: when the install is complete, the output from yarn or npm is cleared since it is no longer relevant and the build continues. When an error occurs, it stays up to allow debugging.


return new Promise((resolve, reject) => {
cp.on('error', reject);
cp.on('close', function(code) {
if (code !== 0) {
return reject(new Error(cmd + ' failed.'));
}

logger.clear();
return resolve();
});
});
Expand Down
Loading