Skip to content

Commit

Permalink
refactor: Replace ncjsm/resolve usage with native createRequire
Browse files Browse the repository at this point in the history
  • Loading branch information
medikoo committed Jan 27, 2022
1 parent d403bfc commit d18efc2
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 20 deletions.
6 changes: 3 additions & 3 deletions lib/classes/PluginManager.js
Expand Up @@ -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');
Expand All @@ -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) => {
Expand Down Expand Up @@ -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 });
}
Expand All @@ -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) {
Expand Down
9 changes: 7 additions & 2 deletions lib/cli/resolve-local-serverless-path.js
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/configuration/read.js
Expand Up @@ -5,16 +5,16 @@ 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');

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(
Expand All @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions 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,
});
8 changes: 3 additions & 5 deletions lib/utils/telemetry/generatePayload.js
Expand Up @@ -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');
Expand All @@ -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');

Expand Down Expand Up @@ -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;
}
Expand Down
11 changes: 10 additions & 1 deletion 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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions test/unit/lib/classes/PluginManager.test.js
Expand Up @@ -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');
Expand All @@ -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');
Expand Down Expand Up @@ -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(() => {
Expand Down
4 changes: 3 additions & 1 deletion test/unit/lib/configuration/read.test.js
Expand Up @@ -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', {
Expand Down

0 comments on commit d18efc2

Please sign in to comment.