Merge branch 'dougmoscrop:master' into master

Author: kaeross

README.md

@@ -2,6 +2,10 @@
 
 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.
 
+5.1.0 introduces support for detecting dependencies of files included via package.patterns
+
+This is useful if you are dynamically importing a directory.
+
 As of 5.0.0 this plugin uses the `package.patterns` property. `always` is no longer supported as it should be possible with just package.patterns
 
 > Note: This plugin no longer excludes the `aws-sdk` to stay in line with AWS best practices (bring your own SDK)
@@ -57,7 +61,7 @@ For even smaller function packages, you can also set:
 package:
   individually: true
 ```
-But be warned: Smaller individual functions can still mean a larger overall deployment. (10 functions that are 3 MB each is more net data tranfer and storage than 1 function that is 6 MB)
+But be warned: Smaller individual functions can still mean a larger overall deployment. (10 functions that are 3 MB each is more net data transfer and storage than 1 function that is 6 MB)
 
 ## Dependency caching (Experimental)
 

__tests__/get-dependency-list.js

@@ -12,6 +12,10 @@ const serverless = {
   }
 };
 
+function convertSlashes(paths) {
+  return paths.map(name => name.replaceAll('\\', '/'));
+}
+
 test('includes a deep dependency', (t) => {
   const fileName = path.join(__dirname, 'fixtures', 'thing.js');
 
@@ -66,7 +70,7 @@ test('handles requiring dependency file in scoped package', (t) => {
 test('should handle requires with same relative path but different absolute path', (t) => {
   const fileName = path.join(__dirname, 'fixtures', 'same-relative-require.js');
 
-  const list = getDependencyList(fileName, serverless);
+  const list = convertSlashes(getDependencyList(fileName, serverless));
 
   t.true(list.some(item => item.indexOf(`bar/baz.js`) !== -1));
   t.true(list.some(item => item.indexOf(`foo/baz.js`) !== -1));
@@ -125,8 +129,9 @@ test('caches lookups', (t) => {
   const fileName2 = path.join(__dirname, 'fixtures', 'redundancies-2.js');
 
   const cache = new Set();
-  const list1 = getDependencyList(fileName, serverless, cache);
-  const list2 = getDependencyList(fileName2, serverless, cache);
+  const checkedFiles = new Set();
+  const list1 = getDependencyList(fileName, serverless, checkedFiles, cache);
+  const list2 = getDependencyList(fileName2, serverless, checkedFiles, cache);
 
   t.true(list1.some(item => item.endsWith('local/named/index.js')));
   t.true(list1.some(item => item.endsWith('symlinked.js')));

__tests__/include-dependencies.js

@@ -8,18 +8,19 @@ const sinon = require('sinon');
 
 const IncludeDependencies = require('../include-dependencies.js');
 
-function createTestInstance(serverless, options) {
+function convertSlashes(paths) {
+  return paths.map(name => name.replaceAll('\\', '/'));
+}
+
+function createTestInstance(serverless, options, functions = {a: {}, b: {}}) {
   return new IncludeDependencies(
     _.merge({
       version: '2.32.0',
       config: {
         servicePath: path.join(__dirname, 'fixtures')
       },
       service: {
-        functions: {
-          a: {},
-          b: {}
-        }
+        functions
       }
     }, serverless),
     _.merge({}, options)
@@ -68,6 +69,32 @@ test('createDeploymentArtifacts should call processFunction with function name',
   t.deepEqual(spy.calledWith('b'), true);
 });
 
+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 processFunctionSpy = sinon.stub(instance, 'processFunction');
+  const dependencyListSpy = sinon.stub(instance, 'getDependencyList');
+
+  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);
+});
+
 test('processFunction should exclude node_modules when no package defined', t => {
   const instance = createTestInstance();
 
@@ -79,6 +106,7 @@ test('processFunction should exclude node_modules when no package defined', t =>
   t.deepEqual(instance.serverless.service.package.patterns, ['!node_modules/**']);
 });
 
+
 test('processFunction should add node_modules ignore to package patterns', t => {
   const instance = createTestInstance({
     service: {
@@ -116,7 +144,7 @@ test('processFunction should add to package include', t => {
 
   instance.processFunction('a');
 
-  t.deepEqual(instance.serverless.service.package.patterns, [
+  t.deepEqual(convertSlashes(instance.serverless.service.package.patterns), [
     '!node_modules/**',
     '.something',
     'node_modules/brightspace-auth-validation/index.js',
@@ -156,7 +184,7 @@ test('processFunction should include individually', t => {
     '!node_modules/**',
     '.something',
   ]);
-  t.deepEqual(instance.serverless.service.functions.a.package.patterns, [
+  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'
@@ -220,9 +248,9 @@ test('getDependencies should handle exclude of a file within a dependency', t =>
 test('getDependencies should ignore excludes that do not start with node_modules', t => {
   const instance = createTestInstance();
   const file = path.join(__dirname, 'fixtures', 'thing.js');
-  const dependencies = instance.getDependencies(file, [
+  const dependencies = convertSlashes(instance.getDependencies(file, [
     '**/LICENSE'
-  ]);
+  ]));
 
   t.true(Array.isArray(dependencies));
   t.true(dependencies.length > 0);
@@ -322,6 +350,8 @@ 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);
 });
@@ -332,6 +362,7 @@ test('enables caching', t => {
   });
   const file = path.join(__dirname, 'fixtures', 'thing.js');
   const list1 = instance.getDependencies(file, []);
+  instance.checkedFiles.clear();
   const list2 = instance.getDependencies(file, []);
   t.true(list2.length < list1.length);
 });

get-dependency-list.js

@@ -13,12 +13,13 @@ function ignoreMissing(dependency, optional, peerDependenciesMeta) {
     || peerDependenciesMeta && dependency in peerDependenciesMeta && peerDependenciesMeta[dependency].optional;
 }
 
-module.exports = function(filename, serverless, cache) {
+module.exports = function(filename, serverless, checkedFiles, 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(/\\/, '/'));
@@ -38,7 +39,7 @@ module.exports = function(filename, serverless, cache) {
         if (cache) {
           cache.add(cacheKey);
         }
-  
+
       } else {
         // TODO: should we warn here?
       }
@@ -69,11 +70,12 @@ module.exports = function(filename, serverless, cache) {
   while (localFilesToProcess.length) {
     const currentLocalFile = localFilesToProcess.pop();
 
-    if (filePaths.has(currentLocalFile)) {
+    if (filePaths.has(currentLocalFile) || checkedFiles.has(currentLocalFile)) {
       continue;
     }
 
     filePaths.add(currentLocalFile);
+    checkedFiles.add(currentLocalFile);
 
     precinct.paperwork(currentLocalFile, { includeCore: false }).forEach(dependency => {
       if (dependency.indexOf('.') === 0) {
@@ -122,5 +124,5 @@ module.exports = function(filename, serverless, cache) {
     });
   });
 
-  return Array.from(filePaths);
+  return Array.from(filePaths).map(file => file.replace(/\\/, '/'));
 };

include-dependencies.js

@@ -4,6 +4,7 @@ const path = require('path');
 
 const semver = require('semver');
 const micromatch = require('micromatch');
+const glob = require('glob');
 
 const getDependencyList = require('./get-dependency-list');
 
@@ -32,6 +33,7 @@ module.exports = class IncludeDependencies {
     this.serverless = serverless;
     this.options = options;
     this.cache = new Set();
+    this.checkedFiles = new Set();
 
     const service = this.serverless.service;
     this.individually = service.package && service.package.individually;
@@ -53,6 +55,23 @@ module.exports = class IncludeDependencies {
     for (const functionName in functions) {
       this.processFunction(functionName);
     }
+
+    const files = [...new Set(this.getPatterns().filter(pattern => !pattern.startsWith('!') && !pattern.includes('node_modules'))
+      .map(modulePath => glob.sync(modulePath, {
+          nodir: true,
+          ignore: path.join(modulePath, 'node_modules', '**'),
+          absolute: true
+        })
+      ).flat().map(file => file.replaceAll('\\', '/')))];
+
+    files.forEach(fileName => {
+      if (!this.checkedFiles.has(fileName)) {
+        const dependencies = this.getDependencies(fileName, service.package.patterns);
+        service.package.patterns = union(service.package.patterns, dependencies);
+      }
+    });
+
+    this.checkedFiles.clear();
   }
 
   processFunction(functionName) {
@@ -69,6 +88,11 @@ module.exports = class IncludeDependencies {
     }
   }
 
+  getPatterns() {
+    const service = this.serverless.service;
+    return (service.package && service.package.patterns) || [];
+  }
+
   getPluginOptions() {
     const service = this.serverless.service;
     return (service.custom && service.custom.includeDependencies) || {};
@@ -78,7 +102,7 @@ module.exports = class IncludeDependencies {
     const { service } = this.serverless;
 
     functionObject.package = functionObject.package || {};
-    
+
     const fileName = this.getHandlerFilename(functionObject.handler);
     const dependencies = this.getDependencies(fileName, service.package.patterns);
 
@@ -103,7 +127,7 @@ module.exports = class IncludeDependencies {
 
   getDependencies(fileName, patterns) {
     const servicePath = this.serverless.config.servicePath;
-    const dependencies = this.getDependencyList(fileName);
+    const dependencies = this.getDependencyList(fileName) || [];
     const relativeDependencies = dependencies.map(p => path.relative(servicePath, p));
 
     const exclusions = patterns.filter(p => {
@@ -121,9 +145,9 @@ module.exports = class IncludeDependencies {
     if (!this.individually) {
       const options = this.getPluginOptions();
       if (options && options.enableCaching) {
-        return getDependencyList(fileName, this.serverless, this.cache);
+        return getDependencyList(fileName, this.serverless, this.checkedFiles, this.cache);
       }
     }
-    return getDependencyList(fileName, this.serverless);
+    return getDependencyList(fileName, this.serverless, this.checkedFiles);
   }
 };

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "serverless-plugin-include-dependencies",
-  "version": "5.0.0",
+  "version": "5.1.0",
   "engines": {
     "node": ">=4.0"
   },