Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

package: individually: true generates huge files when using the same handler file for multiple functions #195

Closed
msl-kabo opened this issue Aug 20, 2017 · 16 comments · Fixed by #198
Labels
Milestone

Comments

@msl-kabo
Copy link

This is a Bug Report

Description

For bug reports:

  • What went wrong?
    I ran serverless deploy -s test and checked the code size of the functions in the lambda console. The size had gone up from 326.9 kB to 36.4MB.
  • What did you expect should have happened?
    The size to go down, and differ between the packages
  • What was the config you used?
    serverless.yml
package:
  individually: true
custom:
  webpackIncludeModules: true

webpack.config.js

const UglifyJSPlugin = require('uglifyjs-webpack-plugin')
const slsw = require('serverless-webpack')
const Visualizer = require('webpack-visualizer-plugin')

module.exports = {
  entry: slsw.lib.entries,
  target: 'node',
  externals: ['aws-sdk'],
  plugins: [
    new UglifyJSPlugin(),
    new Visualizer({filename: '../webpack-stats.html'}),
  ]
}
  • What stacktrace or error message from your provider did you see?

Additional Data

  • Serverless-Webpack Version you're using: github master (git://github.com/elastic-coders/serverless-webpack.git#85560afce2c60cdf6c4ba54392e45e991e0cc595)
  • Webpack version you're using: 3.5.5
  • Serverless Framework Version you're using: 1.20.2
  • Operating System: Debian Stretch
  • Stack Trace (if available):

My serverless.yml has 3 functions, all in the same file, handler.js.

@msl-kabo
Copy link
Author

package:
  individually: false

selection_016

package:
  individually: true

selection_017

@msl-kabo
Copy link
Author

Deploying functions individually seems to work fine though.

package:
  individually: false

serverless deploy function -s test -f functionName produces a small function in the Lamda console.

@HyperBrain
Copy link
Member

HyperBrain commented Aug 20, 2017

It seems there's something special with the first 2 functions. Can you post the compile output for individual packaging for the 3 functions (webpack should in this case compile all 3 functions separately)?
It is important to know what makes the difference (maybe a refence used only in 1+2).

Can you do a serverless package, and check the contents of the generated function zip files in .serverless and compare the packaged node_modules directories? Is there an additional dependency in there?

We use it in multiple of our projects here and never encountered such a strange behavior - with and without individual packaging (the difference is, that we use node-externals to handle the externals for the webpack compile). Node-externals let's you also define a whitelist for external modules that you want to compile into your code.

@msl-kabo
Copy link
Author

Output of serverless package:

$ SLS_DEBUG=* ./node_modules/.bin/serverless package -s test
Serverless: WARNING: Plugin ServerlessWebpack uses deprecated hook before:deploy:createDeploymentArtifacts,
                     use package:createDeploymentArtifacts hook instead
Serverless: WARNING: Plugin ServerlessWebpack uses deprecated hook after:deploy:createDeploymentArtifacts,
                     use package:createDeploymentArtifacts hook instead
Serverless: Bundling with Webpack...
Time: 12818ms
     Asset    Size  Chunks                    Chunk Names
handler.js  704 kB       0  [emitted]  [big]  handler
 [115] ./node_modules/lodash/_arrayMap.js 556 bytes {0} [built]
 [187] ./handler.js 7.96 kB {0} [built]
 [188] ./node_modules/swagger-client/dist/index.js 35.2 kB {0} [built]
 [440] ./node_modules/identity-jwt-nodepackage/index.js 2.15 kB {0} [built]
 [455] ./swagger.json 55 kB {0} [built]
 [456] ./ServiceError.js 196 bytes {0} [built]
 [457] ./node_modules/promise-timeout/index.js 965 bytes {0} [built]
 [458] ./node_modules/promise-pipe/src/PromisePipe.js 24.4 kB {0} [built]
 [460] ./node_modules/json-stringify-safe/stringify.js 907 bytes {0} [built]
 [461] ./node_modules/promise-pipe/src/RemoteAPIHandlers.js 2.15 kB {0} [built]
 [462] ./node_modules/lodash/uniqBy.js 1.01 kB {0} [built]
 [463] ./node_modules/lodash/_baseIteratee.js 895 bytes {0} [built]
 [542] ./node_modules/lodash/intersection.js 953 bytes {0} [built]
 [552] ./node_modules/lodash/_castArrayLikeObject.js 381 bytes {0} [built]
 [554] ./node_modules/lodash/flatten.js 489 bytes {0} [built]
    + 542 hidden modules

WARNING in ./node_modules/encoding/lib/iconv-loader.js
9:12-34 Critical dependency: the request of a dependency is an expression
Serverless: Packing external modules:
Unhandled rejection No packages found

Serverless: Packaging service...
Serverless: Excluding development dependencies...
Serverless: Excluding development dependencies...

The Excluding development dependencies part is taking a looong time.

If I then unpack the three zip files, the small one only has package.json and handler.js, which is perfect. Unpacking the other two zip files reveals that they contain the entire repo. Files that are in the root of the repo are in these zip files, together with the folder .webpack. It seems it just packaged up the entire root folder, excluding .serverless.

We're not using nodeExternals because it's the opposite of what we want. We want everything bundled in, only blacklisting stuff we don't want bundled in.

@msl-kabo
Copy link
Author

Running serverless deploy function -s test -f functionA produces two zip-files, functionA.zip and functionC.zip (functionC being the one that gets packaged correctly), with functionA.zip containing the entire repo.

@msl-kabo
Copy link
Author

$ node --version
v6.11.2

@msl-kabo
Copy link
Author

I've put together a repo for you to try out. https://github.com/metocean/test-webpack-individually

git clone, npm i, and SLS_DEBUG=* ./node_modules/.bin/serverless package -s test.

If you unpack the zip files in .serverless afterwards you'll find that some of them contain the entire repo. The file some-random-file.txt is in the root of the repo just to show that it's getting included in the zip even though it clearly has nothing to do with the functions.

@msl-kabo
Copy link
Author

I had a colleague test that repo as well, same issue. He's running Mac.

@HyperBrain
Copy link
Member

HyperBrain commented Aug 21, 2017

@msl-kabo Thanks for the detailed input. I will check out the sample repo and try to find out why it did this strange packaging. I remember to have seen a similar issue in the past where something was wrong in the configuration. I'll let you know as soon as I have analyzed it.

@HyperBrain
Copy link
Member

HyperBrain commented Aug 21, 2017

@msl-kabo I reproduced the issue. It seems that the multi compile got broken by one of the last commits (or the merge). The error is, that the plugin does not compile the 3 functions individually anymore and Serverless packages the last two (which is completely crappy).
I will have a look and fix it.

It seems to happen when multiple functions are different exports of the same handler file. If the handlers are separated into different files, it works.

For testing I just duplicated the handler file and referenced a different one from each function definition. This leads to the correct multi-compile:

handler.js  1.41 kB       0  [emitted]  handler
   [0] ./handler.js 1.53 kB {0} [built]
   [1] external "swagger-client" 42 bytes {0} [not cacheable]
   [2] external "promise-timeout" 42 bytes {0} [not cacheable]
   [3] external "promise-pipe" 42 bytes {0} [not cacheable]
Time: 565ms
      Asset     Size  Chunks             Chunk Names
handler2.js  1.41 kB       0  [emitted]  handler2
   [0] ./handler2.js 1.53 kB {0} [built]
   [1] external "swagger-client" 42 bytes {0} [not cacheable]
   [2] external "promise-timeout" 42 bytes {0} [not cacheable]
   [3] external "promise-pipe" 42 bytes {0} [not cacheable]
Time: 561ms
      Asset     Size  Chunks             Chunk Names
handler3.js  1.41 kB       0  [emitted]  handler3
   [0] ./handler3.js 1.53 kB {0} [built]
   [1] external "swagger-client" 42 bytes {0} [not cacheable]
   [2] external "promise-timeout" 42 bytes {0} [not cacheable]
   [3] external "promise-pipe" 42 bytes {0} [not cacheable]

This proves that the bug is limited to the case when more than one function is exported by the same file.
I'll update the bug description accordingly.

The output zip files are all of the same size now (which is a consequence of using node-externals here and not packaging the libraries themselves). This is ok for the sample project setup:

-rw-r--r-- 1 Frank 197609 2827974 Aug 21 13:38 functionA.zip
-rw-r--r-- 1 Frank 197609 2827976 Aug 21 13:38 functionB.zip
-rw-r--r-- 1 Frank 197609 2827976 Aug 21 13:38 functionC.zip

I will add the modules to node-external's whitelist as soon as the bug is fixed and run the test again. Then the zip sizes should be different according to the specific functions used out of the modules.

@HyperBrain HyperBrain added the bug label Aug 21, 2017
@HyperBrain HyperBrain added this to the 3.0.0-rc.2 milestone Aug 21, 2017
@HyperBrain HyperBrain changed the title package: individually: true generates huge files package: individually: true generates huge files when using the same handler file for multiple functions Aug 21, 2017
@HyperBrain
Copy link
Member

HyperBrain commented Aug 21, 2017

@msl-kabo I will fix the wrong packaging issue, but I see a general flaw in the layout of the project.
Webpack can only set whole files as entries (see here). So, if all of your handlers are exports of the same file, this file will be optimized (with all exports that it has in it). This is not optimizable on a function level though.

The benefit from Webpacks optimization is only usable, if you separate the handler layer from the business logic, i.e. you should create 3 handlers for the functions that call the specific export from the lib files. Then Webpack will analyze each handler separately and only use the code used by each specific handler that is accessed from the handlers.

Sample:

# serverless.yml
functions:
  funcA:
    handler: handlers/funcA.handler
  funcB:
    handler: handlers/funcB.handler
  funcC:
    handler: handlers/funcC.handler

File system:

  handlers/
    funcA.js
    funcB.js
    funcC.js
  lib/
    handler.js

The handlers now export a handler function and each handler calls into lib/handler.functionX.
Now Webpack can optimize the whole code correctly.

@HyperBrain
Copy link
Member

It is fixed in the attached PR, but consider changing the project layout as mentioned above ;-)

@msl-kabo
Copy link
Author

Thanks, I hadn't thought about the project layout, great feedback.
Nice job!

@brownbl1
Copy link

brownbl1 commented Jan 27, 2018

I know this is an old issue, but I've been trying to find details on how to resolve the warning that @msl-kabo had in his output here. I'm getting the same thing, and was wondering if you were able to resolve it?

WARNING in ./node_modules/encoding/lib/iconv-loader.js
9:12-34 Critical dependency: the request of a dependency is an expression
 @ ./node_modules/encoding/lib/iconv-loader.js
 @ ./node_modules/encoding/lib/encoding.js
 @ ./node_modules/node-fetch/lib/body.js
 @ ./node_modules/node-fetch/index.js
 @ ./node_modules/expo-server-sdk/build/ExpoClient.js
 @ ./handler.js

Ignoring the warning seems to work fine, but I'm wondering what it means. A google search turned up some similar issues, but nothing related to this plugin (though I'm not sure it's related to this plugin either).

Using IgnorePlugin broke the function, and some of the other solutions out there were more hacks than anything else.

Any thoughts appreciated!

@HyperBrain
Copy link
Member

@itslittlejohn This warning, emitted by webpack, means that Webpack cannot determine a dependency, because somewhere in the code there is a dynamic require, i.e. a call to require() where the arrgument is an expression, that is evaluated at runtime.
It is a general webpack warning and has nothing to do with the plugin, but with the source code that it compiles. In your case, the iconv-loader somewhere does a dynamic require (I assume to load a file according to a locale, that is evaluated at runtime).

@brownbl1
Copy link

Thanks! Makes sense. I'll just continue to ignore it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants