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

fix: honour package.pattern over internally set exclusion list #339

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

codingnuclei
Copy link
Contributor

Hey,

This PR enables the package.pattern to behave such that the user has the final say over what files are packaged.

Currently a list of predefined files are always excluded from the final zip, which is not great from a flexibility point of view #315

const excludedFilesDefault = ['package-lock.json', 'pnpm-lock.yaml', 'yarn.lock', 'package.json'];

Further more currently the files returned from the pattern glob are added to an include list...

const includedFiles = [...packageFiles, ...functionFiles];

... so although I may have used a glob like !**/*.md, those files will still be included into the final zip as the filterFilesForZipPackage will return true for any file that does not meet an exclusion criteria.

Taking the view that what is defined in both package.pattern and function.pattern as what files the user wants then we can remove files as needed.

Let me know what you think :)

Considerations:

  • Some perf improvements should be seen in larger repos as one use of globby has been removed.
  • The glob search is ordered, so that a user can override the excludedFilesDefault.
  • To keep this a none breaking change - the package.json is still being excluded by default. Users will manually have to include it via patterns. In the future this could be reversed.

@codingnuclei codingnuclei force-pushed the feature/all_glob_pattern_matching_from_user branch from 3d3fb50 to 55a0675 Compare July 18, 2022 18:12
@OlivierCuyp
Copy link

@floydspace is there a chance for this PR to be merge ?
I have a logger that takes information from the package.json when instanciating, so that all logs contains the name of the package and its version.
I couldn't understand why, even with the following esbuild config I didn't get the package.json in the lambda:

const copyStaticFiles = require('esbuild-copy-static-files');

module.exports = (serverless) => ({
  bundle: true,
  minify: true,
  format: 'cjs',
  logLevel: 'info',
  platform: 'node',
  sourcemap: true,
  tsconfig: 'tsconfig.json',
  watch: {
    pattern: ['src/**/*.ts'],
    ignore: ['.serverless/**/*', '.build']
  },
  plugins: [
    copyStaticFiles({
      src: './package.json',
      dest: '.esbuild/.build/package.json',
      dereference: true,
      recursive: false
    }),
    copyStaticFiles({
      src: './src/docs',
      dest: '.esbuild/.build/src/docs',
      dereference: true,
      recursive: true
    })
  ]
});

Until I read the code and the felt on this issue...

@OlivierCuyp
Copy link

Btw, I'm not event sure why we should exclude any files from the build folder.
All package.* files won't be there except if it is requested by the user...

@paulgalow
Copy link

Amazon Inspector can now scan Lambda functions for vulnerabilities. However it looks like it is using package.json and package-lock.json to achieve this for Node.js functions. So having an option to manually include those files in the ZIP artifacts to be deployed would be really useful.

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