Repository: serverless-heaven/serverless-webpack (js)

Issue:
External modules are not packaged with webpack 5
<!--
1. Please check if an issue already exists so there are no duplicates
2. Fill out the whole template so we have a good overview on the issue
3. Do not remove any section of the template. If something is not applicable leave it empty but leave it in the Issue
4. Please follow the template, otherwise we'll have to ask you to update it
-->

# This is a Bug Report

## Description

Since updating to the release version of webpack 5, it seems like external modules are no longer packaged. I investigated a bit and in `packExternalModules#getExternalModules` I don't see the externals I specify in my webpack config. I checked webpack 5 release notes but could not find a not about that and how to fix it. As a workaround I was able to use `stats.compilation.modules` instead of `chunks` and it does include the externals. Not sure if this is a proper fix though.

Similar or dependent issue(s):
N/A

## Additional Data

* ***Serverless-Webpack Version you're using***: 5.3.5
* ***Webpack version you're using***: 5.1.3
* ***Serverless Framework Version you're using***: 2.8.0
* ***Operating System***: MacOS 10.15.7
* ***Stack Trace (if available)***: N/A


Pull request:
Fix external modules with webpack 5
<!--
1. Do not remove any section of the template. If something is not applicable leave it empty but leave it in the PR
2. Please follow the template, otherwise we'll have to ask you to update it and it will take longer until your PR is merged
-->

## What did you implement:

Closes #651

<!--
Briefly describe the feature if no issue exists for this PR. If possible only
submit PRs for existing issues. If the PR is trivial (like doc changes or simple
code fixes) it can be submitted without a related issue, but as soon as it adds
or changes functionality, a related issue should be present.
-->

Seems like the issue happens because external modules are now concatenable (see https://github.com/webpack/webpack/releases/tag/v5.0.0-beta.31). What I found is using compilation.modules instead of chunks.modulesIterable includes all modules before concatenation so the issue does not happen. This also removed usages of deprecated module.issuer api.

## How did you implement it:

- Use compilation.modules instead of chunks.moduleIterable
- Use ModuleGraph api instead of deprecated module.issuer

## How can we verify it:

<!--
Add any applicable config, commands, screenshots or other resources
to make it easy for us to verify this works. The easier you make it for us
to review a PR, the faster we can review and merge it.

Examples:
* Step by step description, how to verify
* Screenshots - Showing the difference between your output and the master
* Other - Anything else that comes to mind to help us evaluate
-->

Using webpack 5, package an app with some external packages, make sure these packages are included in the function zip.

```js
// webpack.config.js
{
  ...
  externals: ['sharp', 'mjml']
}
```

Note: This wasn't tested with webpack 4 so if anyone has a webpack 4 setup and wants to test that would be awesome.

## Todos:

- [x] Write tests
- [ ] Write documentation
- [x] Fix linting errors
- [x] Make sure code coverage hasn't dropped
- [x] Provide verification config / commands / resources
- [x] Enable "Allow edits from maintainers" for this PR
- [x] Update the messages below

***Is this ready for review?:*** YES
***Is it a breaking change?:*** NO


Patch:
diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js
index 96b4a4c..d3bf4a8 100644
--- a/lib/packExternalModules.js
+++ b/lib/packExternalModules.js
@@ -130,7 +130,7 @@ function getProdModules(externalModules, packagePath, dependencyGraph, forceExcl
         const originInfo = _.get(dependencyGraph, 'dependencies', {})[module.origin] || {};
         moduleVersion = _.get(_.get(originInfo, 'dependencies', {})[module.external], 'version');
         if (!moduleVersion) {
-          // eslint-disable-next-line lodash/path-style 
+          // eslint-disable-next-line lodash/path-style
           moduleVersion = _.get(dependencyGraph, [ 'dependencies', module.external, 'version' ]);
         }
         if (!moduleVersion) {
@@ -182,36 +182,42 @@ function isExternalModule(module) {
   return _.startsWith(module.identifier(), 'external ') && !isBuiltinModule(getExternalModuleName(module));
 }
 
+/**
+ * Gets the module issuer. The ModuleGraph api does not exists in webpack@4
+ * so falls back to using module.issuer.
+ */
+function getIssuerCompat(moduleGraph, module) {
+  if (moduleGraph) {
+    return moduleGraph.getIssuer(module);
+  }
+
+  return module.issuer;
+}
+
 /**
  * Find the original module that required the transient dependency. Returns
  * undefined if the module is a first level dependency.
+ * @param {Object} moduleGraph - Webpack module graph
  * @param {Object} issuer - Module issuer
  */
-function findExternalOrigin(issuer) {
+function findExternalOrigin(moduleGraph, issuer) {
   if (!_.isNil(issuer) && _.startsWith(issuer.rawRequest, './')) {
-    return findExternalOrigin(issuer.issuer);
+    return findExternalOrigin(moduleGraph, getIssuerCompat(moduleGraph, issuer));
   }
   return issuer;
 }
 
-function getExternalModules(stats) {
-  if (!stats.compilation.chunks) {
-    return [];
-  }
+function getExternalModules({ compilation }) {
   const externals = new Set();
-  for (const chunk of stats.compilation.chunks) {
-    if (!chunk.modulesIterable) {
-      continue;
-    }
-
-    // Explore each module within the chunk (built inputs):
-    for (const module of chunk.modulesIterable) {
-      if (isExternalModule(module)) {
-        externals.add({
-          origin: _.get(findExternalOrigin(module.issuer), 'rawRequest'),
-          external: getExternalModuleName(module)
-        });
-      }
+  for (const module of compilation.modules) {
+    if (isExternalModule(module)) {
+      externals.add({
+        origin: _.get(
+          findExternalOrigin(compilation.moduleGraph, getIssuerCompat(compilation.moduleGraph, module)),
+          'rawRequest'
+        ),
+        external: getExternalModuleName(module)
+      });
     }
   }
   return Array.from(externals);


Test patch:
diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js
index 6999994..c80a441 100644
--- a/tests/packExternalModules.test.js
+++ b/tests/packExternalModules.test.js
@@ -20,19 +20,19 @@ chai.use(require('sinon-chai'));
 
 const expect = chai.expect;
 
-class ChunkMock {
-  constructor(modules) {
-    this._modules = modules;
-  }
-
-  get modulesIterable() {
-    return this._modules;
+class WebpackModuleGraphMock {
+  getIssuer(module) {
+    return module.issuer;
   }
 }
 
-class ChunkMockNoModulesIterable {
+class WebpackCompilationMock {
   constructor(modules) {
-    this._modules = modules;
+    this.moduleGraph = new WebpackModuleGraphMock();
+    this.modules = modules;
+    this.compiler = {
+      outputPath: '/my/Service/Path/.webpack/service'
+    };
   }
 }
 
@@ -137,187 +137,151 @@ describe('packExternalModules', () => {
     const stats = {
       stats: [
         {
-          compilation: {
-            chunks: [
-              new ChunkMock([
-                {
-                  identifier: _.constant('"crypto"')
-                },
-                {
-                  identifier: _.constant('"uuid/v4"')
-                },
-                {
-                  identifier: _.constant('"mockery"')
-                },
-                {
-                  identifier: _.constant('"@scoped/vendor/module1"')
-                },
-                {
-                  identifier: _.constant('external "@scoped/vendor/module2"')
-                },
-                {
-                  identifier: _.constant('external "uuid/v4"')
-                },
-                {
-                  identifier: _.constant('external "bluebird"')
-                }
-              ]),
-              new ChunkMockNoModulesIterable([])
-            ],
-            compiler: {
-              outputPath: '/my/Service/Path/.webpack/service'
+          compilation: new WebpackCompilationMock([
+            {
+              identifier: _.constant('"crypto"')
+            },
+            {
+              identifier: _.constant('"uuid/v4"')
+            },
+            {
+              identifier: _.constant('"mockery"')
+            },
+            {
+              identifier: _.constant('"@scoped/vendor/module1"')
+            },
+            {
+              identifier: _.constant('external "@scoped/vendor/module2"')
+            },
+            {
+              identifier: _.constant('external "uuid/v4"')
+            },
+            {
+              identifier: _.constant('external "bluebird"')
             }
-          }
+          ])
         }
       ]
     };
     const noExtStats = {
       stats: [
         {
-          compilation: {
-            chunks: [
-              new ChunkMock([
-                {
-                  identifier: _.constant('"crypto"')
-                },
-                {
-                  identifier: _.constant('"uuid/v4"')
-                },
-                {
-                  identifier: _.constant('"mockery"')
-                },
-                {
-                  identifier: _.constant('"@scoped/vendor/module1"')
-                }
-              ])
-            ],
-            compiler: {
-              outputPath: '/my/Service/Path/.webpack/service'
+          compilation: new WebpackCompilationMock([
+            {
+              identifier: _.constant('"crypto"')
+            },
+            {
+              identifier: _.constant('"uuid/v4"')
+            },
+            {
+              identifier: _.constant('"mockery"')
+            },
+            {
+              identifier: _.constant('"@scoped/vendor/module1"')
             }
-          }
+          ])
         }
       ]
     };
     const statsWithFileRef = {
       stats: [
         {
-          compilation: {
-            chunks: [
-              new ChunkMock([
-                {
-                  identifier: _.constant('"crypto"')
-                },
-                {
-                  identifier: _.constant('"uuid/v4"')
-                },
-                {
-                  identifier: _.constant('"mockery"')
-                },
-                {
-                  identifier: _.constant('"@scoped/vendor/module1"')
-                },
-                {
-                  identifier: _.constant('external "@scoped/vendor/module2"')
-                },
-                {
-                  identifier: _.constant('external "uuid/v4"')
-                },
-                {
-                  identifier: _.constant('external "localmodule"')
-                },
-                {
-                  identifier: _.constant('external "bluebird"')
-                }
-              ])
-            ],
-            compiler: {
-              outputPath: '/my/Service/Path/.webpack/service'
+          compilation: new WebpackCompilationMock([
+            {
+              identifier: _.constant('"crypto"')
+            },
+            {
+              identifier: _.constant('"uuid/v4"')
+            },
+            {
+              identifier: _.constant('"mockery"')
+            },
+            {
+              identifier: _.constant('"@scoped/vendor/module1"')
+            },
+            {
+              identifier: _.constant('external "@scoped/vendor/module2"')
+            },
+            {
+              identifier: _.constant('external "uuid/v4"')
+            },
+            {
+              identifier: _.constant('external "localmodule"')
+            },
+            {
+              identifier: _.constant('external "bluebird"')
             }
-          }
+          ])
         }
       ]
     };
     const statsWithDevDependency = {
       stats: [
         {
-          compilation: {
-            chunks: [
-              new ChunkMock([
-                {
-                  identifier: _.constant('"crypto"')
-                },
-                {
-                  identifier: _.constant('"uuid/v4"')
-                },
-                {
-                  identifier: _.constant('external "eslint"')
-                },
-                {
-                  identifier: _.constant('"mockery"')
-                },
-                {
-                  identifier: _.constant('"@scoped/vendor/module1"')
-                },
-                {
-                  identifier: _.constant('external "@scoped/vendor/module2"')
-                },
-                {
-                  identifier: _.constant('external "uuid/v4"')
-                },
-                {
-                  identifier: _.constant('external "localmodule"')
-                },
-                {
-                  identifier: _.constant('external "bluebird"')
-                }
-              ])
-            ],
-            compiler: {
-              outputPath: '/my/Service/Path/.webpack/service'
+          compilation: new WebpackCompilationMock([
+            {
+              identifier: _.constant('"crypto"')
+            },
+            {
+              identifier: _.constant('"uuid/v4"')
+            },
+            {
+              identifier: _.constant('external "eslint"')
+            },
+            {
+              identifier: _.constant('"mockery"')
+            },
+            {
+              identifier: _.constant('"@scoped/vendor/module1"')
+            },
+            {
+              identifier: _.constant('external "@scoped/vendor/module2"')
+            },
+            {
+              identifier: _.constant('external "uuid/v4"')
+            },
+            {
+              identifier: _.constant('external "localmodule"')
+            },
+            {
+              identifier: _.constant('external "bluebird"')
             }
-          }
+          ])
         }
       ]
     };
     const statsWithIgnoredDevDependency = {
       stats: [
         {
-          compilation: {
-            chunks: [
-              new ChunkMock([
-                {
-                  identifier: _.constant('"crypto"')
-                },
-                {
-                  identifier: _.constant('"uuid/v4"')
-                },
-                {
-                  identifier: _.constant('"mockery"')
-                },
-                {
-                  identifier: _.constant('"@scoped/vendor/module1"')
-                },
-                {
-                  identifier: _.constant('external "@scoped/vendor/module2"')
-                },
-                {
-                  identifier: _.constant('external "uuid/v4"')
-                },
-                {
-                  identifier: _.constant('external "localmodule"')
-                },
-                {
-                  identifier: _.constant('external "bluebird"')
-                },
-                {
-                  identifier: _.constant('external "aws-sdk"')
-                }
-              ])
-            ],
-            compiler: {
-              outputPath: '/my/Service/Path/.webpack/service'
+          compilation: new WebpackCompilationMock([
+            {
+              identifier: _.constant('"crypto"')
+            },
+            {
+              identifier: _.constant('"uuid/v4"')
+            },
+            {
+              identifier: _.constant('"mockery"')
+            },
+            {
+              identifier: _.constant('"@scoped/vendor/module1"')
+            },
+            {
+              identifier: _.constant('external "@scoped/vendor/module2"')
+            },
+            {
+              identifier: _.constant('external "uuid/v4"')
+            },
+            {
+              identifier: _.constant('external "localmodule"')
+            },
+            {
+              identifier: _.constant('external "bluebird"')
+            },
+            {
+              identifier: _.constant('external "aws-sdk"')
             }
-          }
+          ])
         }
       ]
     };
@@ -1184,33 +1148,26 @@ describe('packExternalModules', () => {
           const peerDepStats = {
             stats: [
               {
-                compilation: {
-                  chunks: [
-                    new ChunkMock([
-                      {
-                        identifier: _.constant('"crypto"')
-                      },
-                      {
-                        identifier: _.constant('"uuid/v4"')
-                      },
-                      {
-                        identifier: _.constant('"mockery"')
-                      },
-                      {
-                        identifier: _.constant('"@scoped/vendor/module1"')
-                      },
-                      {
-                        identifier: _.constant('external "bluebird"')
-                      },
-                      {
-                        identifier: _.constant('external "request-promise"')
-                      }
-                    ])
-                  ],
-                  compiler: {
-                    outputPath: '/my/Service/Path/.webpack/service'
+                compilation: new WebpackCompilationMock([
+                  {
+                    identifier: _.constant('"crypto"')
+                  },
+                  {
+                    identifier: _.constant('"uuid/v4"')
+                  },
+                  {
+                    identifier: _.constant('"mockery"')
+                  },
+                  {
+                    identifier: _.constant('"@scoped/vendor/module1"')
+                  },
+                  {
+                    identifier: _.constant('external "bluebird"')
+                  },
+                  {
+                    identifier: _.constant('external "request-promise"')
                   }
-                }
+                ])
               }
             ]
           };
@@ -1291,33 +1248,26 @@ describe('packExternalModules', () => {
           const peerDepStats = {
             stats: [
               {
-                compilation: {
-                  chunks: [
-                    new ChunkMock([
-                      {
-                        identifier: _.constant('"crypto"')
-                      },
-                      {
-                        identifier: _.constant('"uuid/v4"')
-                      },
-                      {
-                        identifier: _.constant('"mockery"')
-                      },
-                      {
-                        identifier: _.constant('"@scoped/vendor/module1"')
-                      },
-                      {
-                        identifier: _.constant('external "bluebird"')
-                      },
-                      {
-                        identifier: _.constant('external "request-promise"')
-                      }
-                    ])
-                  ],
-                  compiler: {
-                    outputPath: '/my/Service/Path/.webpack/service'
+                compilation: new WebpackCompilationMock([
+                  {
+                    identifier: _.constant('"crypto"')
+                  },
+                  {
+                    identifier: _.constant('"uuid/v4"')
+                  },
+                  {
+                    identifier: _.constant('"mockery"')
+                  },
+                  {
+                    identifier: _.constant('"@scoped/vendor/module1"')
+                  },
+                  {
+                    identifier: _.constant('external "bluebird"')
+                  },
+                  {
+                    identifier: _.constant('external "request-promise"')
                   }
-                }
+                ])
               }
             ]
           };
@@ -1397,18 +1347,11 @@ describe('packExternalModules', () => {
         const transitiveDepStats = {
           stats: [
             {
-              compilation: {
-                chunks: [
-                  new ChunkMock([
-                    {
-                      identifier: _.constant('external "classnames"')
-                    }
-                  ])
-                ],
-                compiler: {
-                  outputPath: '/my/Service/Path/.webpack/service'
+              compilation: new WebpackCompilationMock([
+                {
+                  identifier: _.constant('external "classnames"')
                 }
-              }
+              ])
             }
           ]
         };