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

add the ability to exclude some files from the package #488

Closed

Conversation

jaydp17
Copy link
Contributor

@jaydp17 jaydp17 commented Mar 2, 2019

What did you implement:

Closes #470

It allows the user to include only a subset of files in the package.

For example, it's very handy to exclude sourcemaps from the package.

How did you implement it:

# serverless.yml
custom:
  webpack:
    builtAssetIncludeGlob: "**/*.{js,json}"

☝️this is how we use it.

We were already using glob package and passing ** to include all files, this PR just makes that input to glob package as a configurable value from serverless.yml.

If not set in serverless.yml it defaults to **, including all files.

How can we verify it:

  1. As described above, just add the option in serverless.yml.
  2. Run sls package
  3. Extract the zip file in .serverless and verify that .js.map files are not included in the package

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • 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

@jaydp17 jaydp17 changed the title add option builtAssetIncludeGlob add the ability to exclude some files from the package Mar 2, 2019
@designfrontier
Copy link
Contributor

@jaydp17 do you mind correcting the merge conflicts outlined above? We're working our way through the backlog of pull requests. Thanks!

@hassankhan
Copy link
Contributor

hassankhan commented Apr 8, 2019

Is this PR potentially a duplicate of #471?

Should we perhaps rename the config option to be includeFiles, so as to be consistent with #471?

/cc @designfrontier @HyperBrain @jaydp17

@HyperBrain
Copy link
Member

Can it be that this functionality is quite dangerous? Currently webpack completely controls what is packaged, i.e. what is used is packaged. Interfering with that might render broken deployments, e.g. if you use the json-loader (which you should when using json files) and forget to include the json files.

@jaydp17
Copy link
Contributor Author

jaydp17 commented Apr 9, 2019

@HyperBrain this won't interfere with webpack at all, as this runs after webpack has finished building.
After webpack has outputted whatever assets it wanted to (i.e. bundle.js, bundle.js.map, etc.), this glob is applied to those files.

@designfrontier give me some time, I'll resolve the merge conflicts.

@hassankhan, includeFiles feels ambiguous, without reading the docs and just by looking at the config, you can't be sure which files does it mean.
Thus I tried to be a little descriptive with builtAssetIncludeGlob key, because we want to let the user know that this glob applies on the asset files outputted by webpack.

However, I'm open to suggestions, if builtAssetIncludeGlob doesn't feel right.

@designfrontier
Copy link
Contributor

I agree that includeFiled feels ambiguous and this makes sense as a way to trim down the size of the built lambda.

builtAssetIncludeGlob sounds clunky but also seems reasonable. I can't think of anything better... but I am not great at naming things sometimes.

jaydp17 added 3 commits April 10, 2019 22:57
to allow users to select which files should be included in the package
By default it includes all built assets
@jaydp17 jaydp17 force-pushed the filter-files-by-regex branch from 32c1556 to db50548 Compare April 10, 2019 17:32
@j0k3r
Copy link
Member

j0k3r commented May 10, 2021

We do have an option excludeRegex which remove some fiels from the zip.
https://github.com/serverless-heaven/serverless-webpack#exclude-files-with-regular-expression

Can this solve your problem?

@j0k3r j0k3r added the awaiting reply Awaiting for a reply from the OP label May 10, 2021
@j0k3r j0k3r closed this Nov 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting reply Awaiting for a reply from the OP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to skip sourcemaps from the package
5 participants