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

Support individual packaging with multi-compile #177

Merged
merged 11 commits into from
Aug 5, 2017

Conversation

HyperBrain
Copy link
Member

@HyperBrain HyperBrain commented Jul 30, 2017

What did you implement:

Closes #181
Closes #120
Closes #173
Closes #117
Closes #116
Relates to #60

How did you implement it:

If package.individually: true is set in the service, the individual entry points (functions) are now compiled by using WebPack's multi-compile feature. That means, that each function is optimized separately as well as the externals for every function. The node_modules for each function are then packaged individually, so that every resulting function zip contains only the needed modules and the individually optimized function handler.

Additionally the plugin does not change serverless.config.servicePath anymore, so that other plugins and the framework itself behave as expected and work during any lifecycle event and Serverless does not interfere with the packaging of the optimized node modules anymore.

Further changes

  • Added ESLint as preparation for 3.0.0, so that the implementation is conformant.
  • Added unit tests for the new functionality (as well as for old ones where tests where missing).
  • Added verbose output with --verbose

How can we verify it:

Use a simple service that defines multiple functions, where each function uses a different set of dependencies. Use a webpack config that uses lib.entries for entry and set the package wide individual packaging in the service.

Now run serverless package and check the generated function zip files in the .serverless folder.

I checked it with one of our production projects, including tests with serverless invoke local. Found no issues. The only thing I have to verify is, that deployments without individual packaging still work.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors (not yet implemented)
  • Make sure code coverage hasn't dropped (not yet implemented)
  • Provide verification config / commands / resources
  • [ ] Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

return;
}
if (!includes) {
return BbPromise.resolve();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be

return BbPromise.resolve().return(stats);

or the next module errors out with null for stats.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Will change it.

@johnf
Copy link

johnf commented Jul 31, 2017

@HyperBrain What should be different if this works? More files inside the zip?

@HyperBrain
Copy link
Member Author

HyperBrain commented Jul 31, 2017

No. It will package only the node modules that are used by the function that is being packaged. Previously if you enabled individual packaging, all the packages were the same (it even packaged all functions in all function packages), i.e. it just duplicated the service package file (as with "without the individually" option). The new packaging is visible as soon as you also set "webpackIncludeModules".

The second major improvement is, that now every function is compiled individually, which leads to much better optimization, because every function is optimized separately. This will show up, when you enable tree shaking in WebPack.

And last, it prepares to use manual multi-compiles. The next step will be, that you can emit an array of webpack configurations from your webpack.conf, that will automatically trigger the multi-compiler. The new packaging infrastructure will allow for that without having to add any new implementation - only validate has to be touched to make it work.

Additionally, the issues with missing modules and unstable packaging should be solved as the plugin does not change the internal Serverless servicePath anymore, which led to unpredictable behavior.

@johnf
Copy link

johnf commented Jul 31, 2017

nm. I had wrong config in serverless.yml. I can now see a different zip file per function.

Some of them look wright and have a single js. Others seem to be 8M in size and have the while source inside.

I'll have more time on the weekend to try and work out what the difference is

@HyperBrain
Copy link
Member Author

HyperBrain commented Jul 31, 2017

You can also try to use "lib.entries" for the webpack entry definition that is available since 2.2.0.
This makes sure that the plugin sets the correct entry points.

// webpack.conf
const slsw = require('serverless-webpack');
module.exports= {
  ...
  entry: slsw.lib.entries,
  ...
  output:
    ...
    filename: '[name].js',
    ...
};

This makes the use of the plugin quite easy and eliminates the chances for errors. It could be that the 8MB issue comes from the old manual declaration.

@HyperBrain HyperBrain force-pushed the v3.0.0-individual-packaging branch from 266cdc3 to fe98c76 Compare July 31, 2017 17:01
@HyperBrain
Copy link
Member Author

HyperBrain commented Jul 31, 2017

Switched one of our production services to this branch now. After the latest fixes it packages everything correctly and independently and even works with git dependencies and the serverless-warmup plugin (which creates a separate function dynamically during the package process).

Also works with separate build and deployment (using the --package option) from different servers.

@HyperBrain HyperBrain force-pushed the v3.0.0-individual-packaging branch from e3a5c62 to 007a925 Compare August 1, 2017 20:01
@cwaltken-edrans
Copy link

On this branch I get the following error... Adding underscore as a dependency did not solve the issue...

  Reference Error ----------------------------------------
 
  _ is not defined
 
     For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable.
 
  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Forums:        forum.serverless.com
     Chat:          gitter.im/serverless/serverless
 
  Your Environment Information -----------------------------
     OS:                     linux
     Node Version:           6.11.1
     Serverless Version:     1.18.0

@HyperBrain
Copy link
Member Author

Fixed in a minute... I checked in the wrong file ;-)

@HyperBrain
Copy link
Member Author

@cwaltken-edrans Can you update and try again?

@cwaltken-edrans
Copy link

cwaltken-edrans commented Aug 3, 2017

Thanks for the quick update!

Can you update and try again?

That fixes the lodash error. Still I'm back at this one:

  Type Error ---------------------------------------------
 
  lambda is not a function
 
     For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable.
 
  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Forums:        forum.serverless.com
     Chat:          gitter.im/serverless/serverless
 
  Your Environment Information -----------------------------
     OS:                     linux
     Node Version:           6.11.1
     Serverless Version:     1.18.0

Maybe there is still another dependency missing?

@HyperBrain
Copy link
Member Author

I used the invoke local multiple times today and had no problems so far. It might be a specific configuration in your project. The best way to investigate it would be to have a quite simple test project that reproduces the error.
Can you create an issue for the problem? Then we can handle it there and we could put all information we have into it.

@cwaltken-edrans
Copy link

cwaltken-edrans commented Aug 3, 2017

I used the invoke local multiple times today and had no problems so far. It might be a specific configuration in your project. The best way to investigate it would be to have a quite simple test project that reproduces the error.
Can you create an issue for the problem? Then we can handle it there and we could put all information we have into it.

I've just checked again. I broke the entry function trying to fix the issues I had previously. I reverted the changes and now I can build my function.

Sadly my initial issue is still there. Functions invoked locally don't have access to files bundled with the file-loader it seems. I'll open an issue on that.

@HyperBrain
Copy link
Member Author

Yes, please do that. I did not do in-depth tests yet with different loaders, so there might be still issues in there that have to be fixed. A big thank you for taking the effort to test 🥇

@HyperBrain HyperBrain force-pushed the v3.0.0-individual-packaging branch from 4f69729 to dd74b14 Compare August 5, 2017 10:16
@HyperBrain HyperBrain force-pushed the v3.0.0-individual-packaging branch from dd74b14 to 017cb20 Compare August 5, 2017 10:39
@HyperBrain HyperBrain merged commit 5a4b861 into v3.0.0 Aug 5, 2017
@HyperBrain HyperBrain mentioned this pull request Aug 5, 2017
@HyperBrain HyperBrain deleted the v3.0.0-individual-packaging branch September 9, 2017 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants