diff --git a/bin/cli b/bin/cli index eb173971b4..78d1347106 100755 --- a/bin/cli +++ b/bin/cli @@ -403,8 +403,6 @@ async function runDev () { async function runServe () { warnIfNpmStart(argv, process.env) process.env.NODE_ENV = process.env.NODE_ENV || 'production' - const { waitForPackagesCache } = require('../lib/plugins/packages') - await waitForPackagesCache() require('../lib/build.js').generateAssetsSync() require('../listen-on-port') } diff --git a/lib/build.test.js b/lib/build.test.js index adcff14c40..3e1d6c2b98 100644 --- a/lib/build.test.js +++ b/lib/build.test.js @@ -5,6 +5,16 @@ const sass = require('sass') const { setPackagesCache } = require('./plugins/packages') const { sassFunctions } = require('./build') +jest.mock('../package.json', () => { + return { + dependencies: { + '@govuk-prototype-kit/common-templates': '1.0.0', + "available-installed-plugin": "^1.0.0", + "available-prerelease-plugin": "2.0.0-rc.0" + } + } +}) + describe('build', () => { describe('sassFunctions', () => { describe('plugin-version-satisfies', () => { diff --git a/lib/dev-server.js b/lib/dev-server.js index 1c530a2511..a5ae807afa 100644 --- a/lib/dev-server.js +++ b/lib/dev-server.js @@ -29,7 +29,6 @@ const { } = require('./utils/paths') const fs = require('fs') const { logPerformanceSummaryOnce, startPerformanceTimer, endPerformanceTimer } = require('./utils/performance') -const { waitForPackagesCache } = require('./plugins/packages') // Build watch and serve async function runDevServer () { @@ -37,9 +36,6 @@ async function runDevServer () { let startupError try { - // Wait for the package cache to be built before doing anything - // to ensure that `pluginVersionSatisfies` runs against accurate data - await waitForPackagesCache() generateAssetsSync() generateCssSync() await utils.waitUntilFileExists(path.join(publicCssDir, 'application.css'), 5000) diff --git a/lib/manage-prototype-handlers.js b/lib/manage-prototype-handlers.js index 661f3414fa..40d09b8239 100644 --- a/lib/manage-prototype-handlers.js +++ b/lib/manage-prototype-handlers.js @@ -21,7 +21,6 @@ const { getAllPackages, getDependentPackages, getDependencyPackages, - waitForPackagesCache, pluginVersionSatisfies } = require('./plugins/packages') @@ -148,15 +147,6 @@ function developmentOnlyMiddleware (req, res, next) { } } -// Middleware to ensure pages load when plugin cache has been initially loaded -async function pluginCacheMiddleware (req, res, next) { - await Promise.race([ - waitForPackagesCache(), - new Promise((resolve) => setTimeout(resolve, 1000)) - ]) - next() -} - const managementLinks = [ { text: 'Manage your prototype', @@ -775,7 +765,6 @@ module.exports = { getPasswordHandler, postPasswordHandler, developmentOnlyMiddleware, - pluginCacheMiddleware, getHomeHandler, getTemplatesHandler, getTemplatesViewHandler, diff --git a/lib/manage-prototype-routes.js b/lib/manage-prototype-routes.js index 42c824c03e..dcf9cef579 100644 --- a/lib/manage-prototype-routes.js +++ b/lib/manage-prototype-routes.js @@ -23,7 +23,6 @@ const { postPluginsModeMiddleware, postPluginsModeHandler, postPluginsStatusHandler, - pluginCacheMiddleware, postPluginsHandler } = require('./manage-prototype-handlers') const { packageDir, projectDir } = require('./utils/paths') @@ -51,8 +50,6 @@ router.post('/password', postPasswordHandler) // view when the prototype is not running in development router.use(developmentOnlyMiddleware) -router.use(pluginCacheMiddleware) - router.get('/', getHomeHandler) router.get('/templates', getTemplatesHandler) diff --git a/lib/plugins/packages.js b/lib/plugins/packages.js index 8c5e116ccd..eed558d7ce 100644 --- a/lib/plugins/packages.js +++ b/lib/plugins/packages.js @@ -17,13 +17,16 @@ const { getConfigForPackage } = require('../utils/requestHttps') const { getProxyPluginConfig } = require('./plugin-utils') const { sortByObjectKey, hasNewVersion } = require('../utils') -let packageTrackerInterval - const packagesCache = {} +let packagesCacheLastFetchTimeMs = 0; +const packagesCacheMaxAgeMs = 60 * 1000 // 1 minute -async function startPackageTracker () { - await updatePackagesInfo() - packageTrackerInterval = setInterval(updatePackagesInfo, 36000) +async function getOrFetchPackagesCache() { + if (Date.now() - packagesCacheLastFetchTimeMs > packagesCacheMaxAgeMs) { + await updatePackagesInfo() + packagesCacheLastFetchTimeMs = Date.now() + } + return packagesCache } async function updatePackagesInfo () { @@ -176,10 +179,6 @@ function packageNameSort (pkgA, pkgB) { * NOTE: Treats pre-releases version numbers as if they were the actual version * to facilitate testing pre-releases * - * WARNING: Needs to be run after the package cache is built. - * Use `waitForPackageCache` to make sure this function - * is called once the cache is ready. - * * @param {string} pluginName * @param {string} semverRange * @@ -187,25 +186,21 @@ function packageNameSort (pkgA, pkgB) { * @internal */ function pluginVersionSatisfies (pluginName, semverRange) { - if (!Object.keys(packagesCache).length) { - throw new Error("The package cache need to be populated before checking plugins' version range") - } - - const plugin = packagesCache[pluginName] + // No need to do a network fetch to npm for the installed version, it will be in package.json + // This will avoid a network request every time the app launches. + // We only need to fetch from npm if the package management pages are loaded + const installedVersion = projectPackage?.dependencies?.[pluginName] - if (!plugin?.pluginConfig) { + if (!installedVersion) { return false } // Use `coerce` to treat pre-releases like if they were the actual package - return semver.satisfies(semver.coerce(plugin.installedVersion), semverRange) + return semver.satisfies(semver.coerce(installedVersion), semverRange) } async function getInstalledPackages () { - if (!Object.keys(packagesCache).length) { - await startPackageTracker() - } - await waitForPackagesCache() + const packagesCache = await getOrFetchPackagesCache(); return Object.values(packagesCache) .filter(({ installed }) => installed) .sort(packageNameSort) @@ -213,34 +208,12 @@ async function getInstalledPackages () { } async function getAllPackages () { - if (!Object.keys(packagesCache).length) { - await startPackageTracker() - } - await waitForPackagesCache() + const packagesCache = await getOrFetchPackagesCache(); return Object.values(packagesCache) .sort(packageNameSort) .reduce(emphasizeBasePlugins, []) } -function sleep (ms) { - return new Promise((resolve) => setTimeout(resolve, ms)) -} - -async function waitForPackagesCache () { - let numberOfRetries = 20 - let waiting = !packageTrackerInterval - while (waiting) { - // If the packageTrackerInterval has been set, then packages cache has been populated at least once - waiting = !packageTrackerInterval && numberOfRetries > 0 - numberOfRetries-- - if (numberOfRetries === 0) { - console.log('Failed to load the package cache') - } else { - await sleep(250) - } - } -} - function normaliseDependencies (dependencies) { return dependencies.map((dependency) => { if (typeof dependency === 'string') { @@ -256,19 +229,13 @@ async function getDependentPackages (packageName, version, mode) { if (mode !== 'uninstall') { return [] } - if (!Object.keys(packagesCache).length) { - await startPackageTracker() - } - await waitForPackagesCache() + const packagesCache = await getOrFetchPackagesCache(); return Object.values(packagesCache) .filter(({ pluginDependencies }) => pluginDependencies?.some((pluginDependency) => pluginDependency === packageName || pluginDependency.packageName === packageName)) } async function getDependencyPackages (packageName, version, mode) { - if (!Object.keys(packagesCache).length) { - await startPackageTracker() - } - await waitForPackagesCache() + await getOrFetchPackagesCache(); const pkg = await lookupPackageInfo(packageName, version) let pluginDependencies = pkg?.pluginDependencies if (version || mode === 'update') { @@ -289,24 +256,19 @@ async function getDependencyPackages (packageName, version, mode) { return dependencyPlugins.filter(({ installed }) => !installed) } -if (!config.getConfig().isTest) { - startPackageTracker() -} - +// Only used for unit tests function setPackagesCache (packagesInfo) { - // Only used for unit tests for (const key in packagesCache) { delete packagesCache[key] } packagesInfo.forEach((packageInfo) => { packagesCache[packageInfo.packageName] = packageInfo }) - packageTrackerInterval = true + packagesCacheLastFetchTimeMs = Date.now() } module.exports = { setPackagesCache, // Only for unit testing purposes - waitForPackagesCache, lookupPackageInfo, getInstalledPackages, getAllPackages, diff --git a/lib/plugins/packages.spec.js b/lib/plugins/packages.spec.js index de34bab3db..e54cf7d11f 100644 --- a/lib/plugins/packages.spec.js +++ b/lib/plugins/packages.spec.js @@ -26,7 +26,9 @@ const registryUrl = 'https://registry.npmjs.org/' jest.mock('../../package.json', () => { return { dependencies: { - '@govuk-prototype-kit/common-templates': '1.0.0' + '@govuk-prototype-kit/common-templates': '1.0.0', + "available-installed-plugin": "^1.0.0", + "available-prerelease-plugin": "2.0.0-rc.0" } } }) @@ -117,59 +119,20 @@ describe('packages', () => { }) describe('pluginVersionSatisfies', () => { - describe('with an empty cache', () => { - it('throws an error', () => { - expect(() => pluginVersionSatisfies('available-installed-plugin', '>=1.0.0')) - .toThrow("The package cache need to be populated before checking plugins' version range") - }) - }) - describe('with a populated cache', () => { - const availableInstalledPackage = { - packageName: 'available-installed-package', - installed: true, - installedVersion: '1.0.0' - } - const availableInstalledPlugin = { - packageName: 'available-installed-plugin', - installed: true, - installedVersion: '1.0.0', - pluginConfig: {} - } - const availableUninstalledPackage = { - packageName: 'available-uninstalled-package', - installed: false - } - const availableInstalledPrereleasePlugin = { - packageName: 'available-prerelease-plugin', - installedVersion: '2.0.0-rc.0', - pluginConfig: {} - } + // See jest.mock('../../package.json') above - beforeEach(() => { - setPackagesCache([ - availableInstalledPackage, - availableInstalledPlugin, - availableUninstalledPackage, - availableInstalledPrereleasePlugin - ]) - }) - - it('returns `true` if plugin is installed and matches the range', () => { - expect(pluginVersionSatisfies('available-installed-plugin', '>=1.0.0')).toBe(true) - }) - it('returns `true` if pre-release for the minimum of the range is installed', () => { - expect(pluginVersionSatisfies('available-prerelease-plugin', '>=2.0.0')).toBe(true) - }) - it('returns `false` if plugin is installed and version does not match the range', () => { - expect(pluginVersionSatisfies('available-installed-plugin', '<1.0.0')).toBe(false) - }) - it('returns `false` if plugin is not installed', () => { - expect(pluginVersionSatisfies('available-uninstalled-package', '>=1.0.0')).toBe(false) - }) - it('returns `false` if the package is installed but not a plugin', () => { - expect(pluginVersionSatisfies('available-installed-package', '>=1.0.0')).toBe(false) - }) + it('returns `true` if plugin is installed and matches the range', () => { + expect(pluginVersionSatisfies('available-installed-plugin', '>=1.0.0')).toBe(true) + }) + it('returns `true` if pre-release for the minimum of the range is installed', () => { + expect(pluginVersionSatisfies('available-prerelease-plugin', '>=2.0.0')).toBe(true) + }) + it('returns `false` if plugin is installed and version does not match the range', () => { + expect(pluginVersionSatisfies('available-installed-plugin', '<1.0.0')).toBe(false) + }) + it('returns `false` if plugin is not installed', () => { + expect(pluginVersionSatisfies('available-uninstalled-package', '>=1.0.0')).toBe(false) }) }) diff --git a/listen-on-port.js b/listen-on-port.js index 96e0d80a5c..312059c4cf 100644 --- a/listen-on-port.js +++ b/listen-on-port.js @@ -1,16 +1,11 @@ // npm dependencies const { runErrorServer } = require('./lib/errorServer') -const { waitForPackagesCache } = require('./lib/plugins/packages.js') const { verboseLog } = require('./lib/utils/verboseLogger') const config = require('./lib/config.js').getConfig(null, false) ;(async () => { try { - // Wait for the package cache to be built before doing anything - // to ensure that `pluginVersionSatisfies` runs against accurate data - await waitForPackagesCache() - // local dependencies const syncChanges = require('./lib/sync-changes') const server = require('./server.js')