Skip to content

Commit

Permalink
Stop calling moduleDoesResolve before installing Cordova npm deps.
Browse files Browse the repository at this point in the history
This fixes the bug where commands like `meteor add-platform ios` would
fail the first time with an error that cordova-lib could not be found,
even though we attempt to install the necessary packages if they have not
already been installed.

To make a very long story short, calling moduleDoesResolve before
installing dependencies like cordova-lib was causing Node.js to cache the
_absence_ of cordova-lib/package.json permanently in the new
packageJsonCache, which cannot be invalidated or cleared by user code:
https://github.com/nodejs/node/blob/f8f20892e944a6c4b52e298528135161d85fcc7a/lib/internal/modules/cjs/loader.js#L245-L255

Although we could potentially propose a change to Node to allow the
packageJsonCache to be invalidated, a more immediate solution is simply to
avoid calling moduleDoesResolve when there's any chance the module will
not resolve. Because we still want to avoid repeatedly installing Cordova
dependencies every time we run a Cordova command, we instead check whether
the necessary dependencies are installed by examining the file system.
  • Loading branch information
Ben Newman committed Mar 11, 2020
1 parent 824b247 commit 2b8834e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 21 deletions.
19 changes: 12 additions & 7 deletions tools/cli/dev-bundle-helpers.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,25 @@
import { pathJoin, getDevBundle } from '../fs/files';
import { installNpmModule, moduleDoesResolve } from '../isobuild/meteor-npm.js';
import { pathJoin, getDevBundle, statOrNull } from '../fs/files';
import { installNpmModule } from '../isobuild/meteor-npm.js';

export function ensureDependencies(deps) {
const devBundleLib = pathJoin(getDevBundle(), 'lib');
const devBundleNodeModules = pathJoin(devBundleLib, 'node_modules');

// Check if each of the requested dependencies resolves, if not
// mark them for installation.
const needToInstall = Object.create(null);
Object.keys(deps).forEach(dep => {
if (!moduleDoesResolve(dep)) {
const pkgDir = pathJoin(devBundleNodeModules, dep);
const pkgStat = statOrNull(pkgDir);
const alreadyInstalled = pkgStat && pkgStat.isDirectory();
if (!alreadyInstalled) {
const versionToInstall = deps[dep];
needToInstall[dep] = versionToInstall;
}
});

const devBundleLib = pathJoin(getDevBundle(), 'lib');

// Install each of the requested modules.
Object.keys(needToInstall)
.forEach(dep => installNpmModule(dep, needToInstall[dep], devBundleLib));
Object.keys(needToInstall).forEach(dep => {
installNpmModule(dep, needToInstall[dep], devBundleLib);
});
}
14 changes: 0 additions & 14 deletions tools/isobuild/meteor-npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -1053,20 +1053,6 @@ var getShrinkwrappedDependencies = function (dir) {
return treeToDependencies(getShrinkwrappedDependenciesTree(dir));
};

const moduleDoesResolve = meteorNpm.moduleDoesResolve = (dep) => {
try {
require.resolve(dep);
} catch (e) {
if (e.code !== 'MODULE_NOT_FOUND') {
throw e;
}

return false;
}

return true;
};

const installNpmModule = meteorNpm.installNpmModule = (name, version, dir) => {
const installArg = utils.isNpmUrl(version)
? version
Expand Down

0 comments on commit 2b8834e

Please sign in to comment.