From d18efc2d43789dc11857d677851ba4b66e09a6a4 Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Fri, 7 Jan 2022 15:53:24 +0100 Subject: [PATCH] refactor: Replace `ncjsm/resolve` usage with native `createRequire` --- lib/classes/PluginManager.js | 6 +++--- lib/cli/resolve-local-serverless-path.js | 9 +++++++-- lib/configuration/read.js | 6 +++--- lib/utils/get-require.js | 9 +++++++++ lib/utils/telemetry/generatePayload.js | 8 +++----- test/mochaPatch.js | 11 ++++++++++- test/unit/lib/classes/PluginManager.test.js | 14 +++++++++----- test/unit/lib/configuration/read.test.js | 4 +++- 8 files changed, 47 insertions(+), 20 deletions(-) create mode 100644 lib/utils/get-require.js diff --git a/lib/classes/PluginManager.js b/lib/classes/PluginManager.js index 7154fabe010..a5a3b6ecb58 100644 --- a/lib/classes/PluginManager.js +++ b/lib/classes/PluginManager.js @@ -2,7 +2,6 @@ const path = require('path'); const config = require('@serverless/utils/config'); -const cjsResolve = require('ncjsm/resolve/sync'); const _ = require('lodash'); const isModuleNotFoundError = require('ncjsm/is-module-not-found-error'); const ServerlessError = require('../serverless-error'); @@ -14,6 +13,7 @@ const generateTelemetryPayload = require('../utils/telemetry/generatePayload'); const processBackendNotificationRequest = require('../utils/processBackendNotificationRequest'); const isTelemetryDisabled = require('../utils/telemetry/areDisabled'); const tokenizeException = require('../utils/tokenize-exception'); +const getRequire = require('../utils/get-require'); const { legacy, log, getPluginWriters } = require('@serverless/utils/log'); const mergeCommands = (target, source) => { @@ -157,7 +157,7 @@ class PluginManager { } } try { - const entryFilePath = cjsResolve(serviceDir, pluginPath).realPath; + const entryFilePath = getRequire(serviceDir).resolve(pluginPath); if (pluginPath.startsWith('./')) { this.localPluginsPaths.push({ resolvedPath: entryFilePath, inputPath: pluginPath }); } @@ -169,7 +169,7 @@ class PluginManager { } // Search in "node_modules" in which framework is placed - return require(cjsResolve(__dirname, pluginPath).realPath); + return require(pluginPath); } resolveServicePlugins(servicePlugs) { diff --git a/lib/cli/resolve-local-serverless-path.js b/lib/cli/resolve-local-serverless-path.js index b9562f658e3..57a2454dd25 100644 --- a/lib/cli/resolve-local-serverless-path.js +++ b/lib/cli/resolve-local-serverless-path.js @@ -2,14 +2,19 @@ const path = require('path'); const memoizee = require('memoizee'); -const resolveSync = require('ncjsm/resolve/sync'); +const { createRequire } = require('module'); // This method should be kept as sync. The reason for it is the fact that // telemetry generation and persistence needs to be run in sync manner // and it depends on this function, either directly or indirectly. module.exports = memoizee(() => { try { - return path.resolve(path.dirname(resolveSync(process.cwd(), 'serverless').realPath), '..'); + return path.resolve( + path.dirname( + createRequire(path.resolve(process.cwd(), 'require-resolver')).resolve('serverless') + ), + '..' + ); } catch { return null; } diff --git a/lib/configuration/read.js b/lib/configuration/read.js index 77e040da922..74eefcafccf 100644 --- a/lib/configuration/read.js +++ b/lib/configuration/read.js @@ -5,8 +5,8 @@ const isPlainObject = require('type/plain-object/is'); const path = require('path'); const fsp = require('fs').promises; const yaml = require('js-yaml'); -const resolveModulePath = require('ncjsm/resolve'); const isModuleNotFoundError = require('ncjsm/is-module-not-found-error'); +const getRequire = require('../utils/get-require'); const spawn = require('child-process-ext/spawn'); const cloudformationSchema = require('@serverless/utils/cloudformation-schema'); const ServerlessError = require('../serverless-error'); @@ -14,7 +14,7 @@ const ServerlessError = require('../serverless-error'); const resolveTsNode = async (serviceDir) => { // 1. If installed aside of a Framework, use it try { - return (await resolveModulePath(__dirname, 'ts-node')).realPath; + return getRequire(__dirname).resolve('ts-node'); } catch (slsDepError) { if (slsDepError.code !== 'MODULE_NOT_FOUND') { throw new ServerlessError( @@ -25,7 +25,7 @@ const resolveTsNode = async (serviceDir) => { // 2. If installed in a service, use it try { - return (await resolveModulePath(serviceDir, 'ts-node')).realPath; + return getRequire(serviceDir).resolve('ts-node'); } catch (serviceDepError) { if (serviceDepError.code !== 'MODULE_NOT_FOUND') { throw new ServerlessError( diff --git a/lib/utils/get-require.js b/lib/utils/get-require.js new file mode 100644 index 00000000000..0ccc87879aa --- /dev/null +++ b/lib/utils/get-require.js @@ -0,0 +1,9 @@ +'use strict'; + +const path = require('path'); +const { createRequire } = require('module'); +const memoize = require('memoizee'); + +module.exports = memoize((dirname) => createRequire(path.resolve(dirname, 'require-resolver')), { + primitive: true, +}); diff --git a/lib/utils/telemetry/generatePayload.js b/lib/utils/telemetry/generatePayload.js index b5885c5c94f..7e79a7ae949 100644 --- a/lib/utils/telemetry/generatePayload.js +++ b/lib/utils/telemetry/generatePayload.js @@ -2,7 +2,6 @@ const path = require('path'); const crypto = require('crypto'); -const resolveSync = require('ncjsm/resolve/sync'); const _ = require('lodash'); const isPlainObject = require('type/plain-object/is'); const isObject = require('type/object/is'); @@ -14,6 +13,7 @@ const { triggeredDeprecations } = require('../logDeprecation'); const isNpmGlobal = require('../npmPackage/isGlobal'); const resolveLocalServerlessPath = require('../../cli/resolve-local-serverless-path'); const resolveIsLocallyInstalled = require('../../utils/is-locally-installed'); +const getRequire = require('../../utils/get-require'); const ci = require('ci-info'); const AWS = require('aws-sdk'); @@ -214,16 +214,14 @@ module.exports = ({ // Since v2.42.0 it's "@serverless/dashboard-plugin" '@serverless/dashboard-plugin': (() => { try { - return require(resolveSync(localServerlessPath, '@serverless/dashboard-plugin/package') - .realPath).version; + return getRequire(localServerlessPath)('@serverless/dashboard-plugin/package').version; } catch { return undefined; } })(), '@serverless/enterprise-plugin': (() => { try { - return require(resolveSync(localServerlessPath, '@serverless/enterprise-plugin/package') - .realPath).version; + return getRequire(localServerlessPath)('@serverless/enterprise-plugin/package').version; } catch { return undefined; } diff --git a/test/mochaPatch.js b/test/mochaPatch.js index 5507a5d438d..759d294c858 100644 --- a/test/mochaPatch.js +++ b/test/mochaPatch.js @@ -1,6 +1,7 @@ 'use strict'; const fs = require('fs'); +const Module = require('module'); // Temporary patch to help tackle peekaboo error, by revelaing fuller stack trace for "fs" errors // https://github.com/serverless/serverless/runs/1740873363 @@ -63,7 +64,15 @@ runnerEmitter.on('runner', (runner) => { resolveInput.clear(); // Ensure to reset cache for local serverless installation resolution // Leaking it across test files may introduce wrong assumptions when result is used for testing - resolveLocalServerless.clear(); + if (resolveLocalServerless._has('data')) { + // As we rely on native require.resolve, we need to ensure that it's cache related to + // tmp homedir is cleared + const homedir = require('os').homedir(); + for (const key of Object.keys(Module._pathCache)) { + if (key.includes(homedir)) delete Module._pathCache[key]; + } + resolveLocalServerless.clear(); + } // Ensure to reset memoization on artifacts generation after each test file run. // Reason is that homedir is automatically cleaned for each test, // therefore eventually built custom resource file is also removed diff --git a/test/unit/lib/classes/PluginManager.test.js b/test/unit/lib/classes/PluginManager.test.js index e9df20f6af3..3403d01c909 100644 --- a/test/unit/lib/classes/PluginManager.test.js +++ b/test/unit/lib/classes/PluginManager.test.js @@ -4,7 +4,6 @@ const chai = require('chai'); const overrideEnv = require('process-utils/override-env'); -const cjsResolve = require('ncjsm/resolve/sync'); const spawn = require('child-process-ext/spawn'); const overrideArgv = require('process-utils/override-argv'); const resolveAwsEnv = require('@serverless/test/resolve-env'); @@ -14,6 +13,7 @@ const CLI = require('../../../../lib/classes/CLI'); const resolveInput = require('../../../../lib/cli/resolve-input'); const Create = require('../../../../lib/plugins/create/create'); const ServerlessError = require('../../../../lib/serverless-error'); +const getRequire = require('../../../../lib/utils/get-require'); const path = require('path'); const fs = require('fs'); @@ -405,18 +405,22 @@ describe('PluginManager', () => { case 'BrokenPluginMock': case 'ServicePluginMock1': case 'ServicePluginMock2': - return { realPath: pluginPath }; + return pluginPath; case './RelativePath/ServicePluginMock2': - return { realPath: `${serviceDir}/RelativePath/ServicePluginMock2` }; + return `${serviceDir}/RelativePath/ServicePluginMock2`; default: - return cjsResolve(directory, pluginPath); + return getRequire(directory).resolve(pluginPath); } }; let restoreEnv; let serviceDir; const PluginManager = proxyquire('../../../../lib/classes/PluginManager', { - 'ncjsm/resolve/sync': resolveStub, + '../utils/get-require': (directory) => { + const resultRequire = require('module').createRequire(path.resolve(directory, 'req')); + resultRequire.resolve = (pluginPath) => resolveStub(directory, pluginPath); + return resultRequire; + }, }); beforeEach(() => { diff --git a/test/unit/lib/configuration/read.test.js b/test/unit/lib/configuration/read.test.js index 337df002bea..2f278383cf6 100644 --- a/test/unit/lib/configuration/read.test.js +++ b/test/unit/lib/configuration/read.test.js @@ -160,11 +160,13 @@ describe('test/unit/lib/configuration/read.test.js', () => { }); it('should reject TS configuration if "ts-node" is not found', async () => { - configurationPath = 'serverless-errored.ts'; + // Test against different service dir, to not fall into cached `require.resolve` value + configurationPath = 'other/serverless-errored.ts'; const configuration = { service: 'test-ts', provider: { name: 'aws' }, }; + await fse.ensureFile(configurationPath); await fsp.writeFile(configurationPath, `module.exports = ${JSON.stringify(configuration)}`); await expect( proxyquire('../../../../lib/configuration/read', {