From 2b8834e92e7c627259026a87bb11d581805d9cbd Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 11 Mar 2020 17:52:43 -0400 Subject: [PATCH] Stop calling moduleDoesResolve before installing Cordova npm deps. 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. --- tools/cli/dev-bundle-helpers.js | 19 ++++++++++++------- tools/isobuild/meteor-npm.js | 14 -------------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/tools/cli/dev-bundle-helpers.js b/tools/cli/dev-bundle-helpers.js index 0a64995b3d..eafcffe932 100644 --- a/tools/cli/dev-bundle-helpers.js +++ b/tools/cli/dev-bundle-helpers.js @@ -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); + }); } diff --git a/tools/isobuild/meteor-npm.js b/tools/isobuild/meteor-npm.js index 30e03cbc82..33ab0ea3b6 100644 --- a/tools/isobuild/meteor-npm.js +++ b/tools/isobuild/meteor-npm.js @@ -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