Skip to content

Commit

Permalink
Merge pull request #95 from MichaelKovalchuk/mkovalchuk/fix-for-indiv…
Browse files Browse the repository at this point in the history 
…idual-packaging

Fix for individual packaging
  • Loading branch information
@dougmoscrop
dougmoscrop authored Jan 11, 2024
2 parents c56f088 + 166686e commit 6c778e9
Show file tree
Hide file tree
Showing 9 changed files with 1,995 additions and 7,275 deletions.
6 changes: 6 additions & 0 deletions .eslintrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,9 @@ extends:

rules:
no-console: 0

parserOptions:
ecmaVersion: 2020
sourceType: "module"
ecmaFeatures:
"js": true
2 changes: 2 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

This is a Serverless plugin that should make your deployed functions smaller. It does this by excluding `node_modules` then individually adds each module file that your handler depends on.

6.0.0 requires Node 18.18 and newer to function. Updated dependencies, fixed small bugs introduced in 5.1.0 version.

5.1.0 introduces support for detecting dependencies of files included via package.patterns

This is useful if you are dynamically importing a directory.
Expand Down
2 changes: 0 additions & 2 deletions __tests__/.eslintrc
View file

This file was deleted.

5 changes: 2 additions & 3 deletions __tests__/get-dependency-list.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,8 @@ test('caches lookups', (t) => {
const fileName2 = path.join(__dirname, 'fixtures', 'redundancies-2.js');

const cache = new Set();
const checkedFiles = new Set();
const list1 = getDependencyList(fileName, serverless, checkedFiles, cache);
const list2 = getDependencyList(fileName2, serverless, checkedFiles, cache);
const list1 = getDependencyList(fileName, serverless, cache);
const list2 = getDependencyList(fileName2, serverless, cache);

t.true(list1.some(item => item.endsWith('local/named/index.js')));
t.true(list1.some(item => item.endsWith('symlinked.js')));
Expand Down
163 changes: 91 additions & 72 deletions __tests__/include-dependencies.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@ const path = require('path');
const _ = require('lodash');
const test = require('ava');
const sinon = require('sinon');

const IncludeDependencies = require('../include-dependencies.js');

const proxyquire = require('proxyquire');
function convertSlashes(paths) {
return paths.map(name => name.replaceAll('\\', '/'));
}

function createTestInstance(serverless, options, functions = {a: {}, b: {}}) {
return new IncludeDependencies(
const getDependencyListStub = (result) => sinon.stub().returns(result);

function createTestInstance({
serverless,
options,
functions = {a: {}, b: {}},
dependencyListStub = null
} = {}) {
return new (proxyquire('../include-dependencies.js', {
...dependencyListStub && { './get-dependency-list': dependencyListStub }
}))(
_.merge({
version: '2.32.0',
config: {
Expand All @@ -28,7 +34,7 @@ function createTestInstance(serverless, options, functions = {a: {}, b: {}}) {
}

test('constructor should throw on older version', t => {
t.throws(() => createTestInstance({ version: '1.12.0' }));
t.throws(() => createTestInstance({ serverless:{ version: '1.12.0' } }));
});

test('constructor should create hooks', t => {
Expand All @@ -41,14 +47,14 @@ test('constructor should create hooks', t => {
});

test('constructor should set properties', t => {
const instance = createTestInstance(undefined, { function: 'some-name' });
const instance = createTestInstance({ options: { function: 'some-name' } });

t.truthy(instance.serverless.service);
t.truthy(instance.options.function);
});

test('functionDeploy should call processFunction with function name', t => {
const instance = createTestInstance(undefined, { function: 'foo' });
const instance = createTestInstance({ options: { function: 'foo' } });
const spy = sinon.stub(instance, 'processFunction');

instance.functionDeploy();
Expand All @@ -71,28 +77,30 @@ test('createDeploymentArtifacts should call processFunction with function name',

test('createDeploymentArtifacts should call getDependencies for patterns files', t => {
const fileName = path.join(__dirname, 'fixtures', 'thing.js').replaceAll('\\', '/');
const instance = createTestInstance({
service: {
provider: {
runtime: 'nodejs18.x',
},
package: {
patterns: [fileName]
const instance = createTestInstance({
serverless: {
service: {
provider: {
runtime: 'nodejs18.x',
},
package: {
patterns: [fileName]
}
}
}
});
});

const processFunctionSpy = sinon.stub(instance, 'processFunction');
const dependencyListSpy = sinon.stub(instance, 'getDependencyList');
const getDependenciesSpy = sinon.stub(instance, 'getDependencies');

instance.createDeploymentArtifacts();

t.deepEqual(processFunctionSpy.calledTwice, true);
t.deepEqual(processFunctionSpy.calledWith('a'), true);
t.deepEqual(processFunctionSpy.calledWith('b'), true);

t.deepEqual(dependencyListSpy.callCount, 1);
t.deepEqual(dependencyListSpy.calledWith(fileName), true);
t.deepEqual(getDependenciesSpy.callCount, 1);
t.deepEqual(getDependenciesSpy.calledWith(fileName), true);
});

test('processFunction should exclude node_modules when no package defined', t => {
Expand All @@ -108,12 +116,14 @@ test('processFunction should exclude node_modules when no package defined', t =>


test('processFunction should add node_modules ignore to package patterns', t => {
const instance = createTestInstance({
service: {
package: {
patterns: ['.something']
const instance = createTestInstance({
serverless: {
service: {
package: {
patterns: ['.something']
}
}
}
}
});

sinon.stub(instance, 'getHandlerFilename').returns('handler.js');
Expand All @@ -125,58 +135,66 @@ test('processFunction should add node_modules ignore to package patterns', t =>
});

test('processFunction should add to package include', t => {
const dependencyListStubReturn = [
path.join('node_modules', 'brightspace-auth-validation', 'index.js'),
path.join('node_modules', 'brightspace-auth-validation', 'node_modules', 'jws', 'index.js'),
];
const dependencyListStub = getDependencyListStub(dependencyListStubReturn);
const instance = createTestInstance({
service: {
provider: {
runtime: 'nodejs14.x',
},
package: {
patterns: ['.something']
serverless: {
service: {
provider: {
runtime: 'nodejs18.x',
},
package: {
patterns: ['.something']
}
}
}
},
dependencyListStub
});

sinon.stub(instance, 'getHandlerFilename').returns('handler.js');
sinon.stub(instance, 'getDependencies').returns([
path.join('node_modules', 'brightspace-auth-validation', 'index.js'),
path.join('node_modules', 'brightspace-auth-validation', 'node_modules', 'jws', 'index.js'),
]);

instance.processFunction('a');

t.deepEqual(convertSlashes(instance.serverless.service.package.patterns), [
'!node_modules/**',
'.something',
'node_modules/brightspace-auth-validation/index.js',
'node_modules/brightspace-auth-validation/node_modules/jws/index.js'
'../../node_modules/brightspace-auth-validation/index.js',
'../../node_modules/brightspace-auth-validation/node_modules/jws/index.js'
]);
});

test('processFunction should include individually', t => {
const dependencyListStubReturn = [
path.join('node_modules', 'brightspace-auth-validation', 'index.js'),
path.join('node_modules', 'brightspace-auth-validation', 'node_modules', 'jws', 'index.js'),
];
const dependencyListStub = getDependencyListStub(dependencyListStubReturn);
const instance = createTestInstance({
service: {
provider: {
runtime: 'nodejs14.x',
},
package: {
individually: true,
patterns: ['.something']
},
functions: {
a: {
package: {
patterns: ['.something-else']
serverless: {
service: {
provider: {
runtime: 'nodejs18.x',
},
package: {
individually: true,
patterns: ['.something']
},
functions: {
a: {
package: {
patterns: ['.something-else']
}
}
}
}
}
},
dependencyListStub
});

sinon.stub(instance, 'getHandlerFilename').returns('handler.js');
sinon.stub(instance, 'getDependencies').returns([
path.join('node_modules', 'brightspace-auth-validation', 'index.js'),
path.join('node_modules', 'brightspace-auth-validation', 'node_modules', 'jws', 'index.js'),
]);

instance.processFunction('a');

Expand All @@ -186,8 +204,8 @@ test('processFunction should include individually', t => {
]);
t.deepEqual(convertSlashes(instance.serverless.service.functions.a.package.patterns), [
'.something-else',
'node_modules/brightspace-auth-validation/index.js',
'node_modules/brightspace-auth-validation/node_modules/jws/index.js'
'../../node_modules/brightspace-auth-validation/index.js',
'../../node_modules/brightspace-auth-validation/node_modules/jws/index.js'
]);
});

Expand Down Expand Up @@ -318,15 +336,15 @@ test('processFunction should handle different runtimes', t => {
package: {
individually: true,
include: ['.something']
}
},
functions: {
a: {},
b: {
runtime: 'nodejs43'
},
functions: {
a: {},
b: {
runtime: 'nodejs43'
},
c: {
runtime: 'nodejs62'
}
c: {
runtime: 'nodejs62'
}
}
});
Expand All @@ -350,19 +368,20 @@ test('disables caching by default', t => {
const instance = createTestInstance();
const file = path.join(__dirname, 'fixtures', 'thing.js');
const list1 = instance.getDependencies(file, []);
instance.checkedFiles.clear(); // clear would be called when through createDeploymentArtifacts

const list2 = instance.getDependencies(file, []);
t.deepEqual(list1, list2);
});

test('enables caching', t => {
const instance = createTestInstance({
service: { custom: { includeDependencies: { enableCaching: true } } }
const instance = createTestInstance({
serverless: {
service: { custom: { includeDependencies: { enableCaching: true } } }
}
});
const cacheEnabled = instance.cacheEnabled;
t.true(cacheEnabled);
const file = path.join(__dirname, 'fixtures', 'thing.js');
const list1 = instance.getDependencies(file, []);
instance.checkedFiles.clear();
const list2 = instance.getDependencies(file, []);
const list1 = instance.getDependencies(file, [], cacheEnabled);
const list2 = instance.getDependencies(file, [], cacheEnabled);
t.true(list2.length < list1.length);
});
17 changes: 6 additions & 11 deletions get-dependency-list.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const path = require('path');

const precinct = require('precinct');
const { paperwork } = require('precinct');
const resolve = require('resolve');
const readPkgUp = require('read-pkg-up');
const requirePackageName = require('require-package-name');
Expand All @@ -13,13 +13,12 @@ function ignoreMissing(dependency, optional, peerDependenciesMeta) {
|| peerDependenciesMeta && dependency in peerDependenciesMeta && peerDependenciesMeta[dependency].optional;
}

module.exports = function(filename, serverless, checkedFiles, cache) {
module.exports = function(filename, serverless, cache) {
const servicePath = serverless.config.servicePath;
const modulePaths = new Set();
const filePaths = new Set();
const modulesToProcess = [];
const localFilesToProcess = [filename];
if (!checkedFiles) checkedFiles = new Set();

function handle(name, basedir, optionalDependencies, peerDependenciesMeta) {
const moduleName = requirePackageName(name.replace(/\\/, '/'));
Expand All @@ -35,7 +34,6 @@ module.exports = function(filename, serverless, checkedFiles, cache) {

if (pkg) {
modulesToProcess.push(pkg);

if (cache) {
cache.add(cacheKey);
}
Expand Down Expand Up @@ -70,15 +68,13 @@ module.exports = function(filename, serverless, checkedFiles, cache) {
while (localFilesToProcess.length) {
const currentLocalFile = localFilesToProcess.pop();

if (filePaths.has(currentLocalFile) || checkedFiles.has(currentLocalFile)) {
if (filePaths.has(currentLocalFile)) {
continue;
}

}
filePaths.add(currentLocalFile);
checkedFiles.add(currentLocalFile);

precinct.paperwork(currentLocalFile, { includeCore: false }).forEach(dependency => {
paperwork(currentLocalFile, { includeCore: false }).forEach(dependency => {
if (dependency.indexOf('.') === 0) {
filePaths.add(dependency);
const abs = resolve.sync(dependency, {
basedir: path.dirname(currentLocalFile)
});
Expand Down Expand Up @@ -124,6 +120,5 @@ module.exports = function(filename, serverless, checkedFiles, cache) {
filePaths.add(moduleFilePath);
});
});

return Array.from(filePaths).map(file => file.replace(/\\/, '/'));
};
Loading

0 comments on commit 6c778e9

Please sign in to comment.