netlify / build

Netlify Build (node process) runs the build command, Build Plugins and bundles Netlify Functions. Can be run in Buildbot or locally using Netlify CLI
https://docs.netlify.com/configure-builds/build-plugins/
MIT License
217 stars 55 forks source link

zipFunction does not bundle @aws-sdk/client-s3 library #5708

Open dsope05 opened 3 months ago

dsope05 commented 3 months ago

Describe the bug

zipFunction w/ esbuild (nodeBundler) does not bundle npm library @aws-sdk/client-s3.

instead, it just includes the import statement in the output bundle file:

also adding @aws-sdk/client-s3 to the externalNodeModules option does not include the node_module in the output folder.

Steps to reproduce

  1. create new folder and npm init, change to "type": "module" in package.json.

  2. install @aws-sdk/client-s3, and @netlify/zip-it-and-ship-it

  3. create simple index.js file that imports and logs @aws-sdk/client-s3 import sdk from '@aws-sdk/client-s3'; const a = () => { console.log('sdk', sdk) } export default a;

  4. create zipit.js file that calls zipFunction with zipConfig

import { zipFunction } from '@netlify/zip-it-and-ship-it'; const zipConfig = { nodeBundler: 'esbuild', externalNodeModules: ['@aws-sdk/client-s3'] }; zipFunction('index.js', 'zip-out', { archiveFormat: 'none', config: { '*': zipConfig, }, }); };

  1. open output file zip-out/index/index.mjs ` import {createRequire as nfyCreateRequire} from "module"; import {fileURLToPath as nfyFileURLToPath} from "url"; import {dirname as _nfyPathDirname} from "path"; let _filename=nfyFileURLToPath(import.meta.url); let dirname=__nfyPathDirname(nfyFileURLToPath(import.meta.url)); let require=___nfyCreateRequire(import.meta.url);

// index.js import sdk from "@aws-sdk/client-s3"; var a = () => { console.log("sdk", sdk); }; var testy_default = a; export { testy_default as default }; `

Result: The @aws-sdk/client-s3 is not bundled, nor is it included in any parallel node_modules in the zip-out folder.

CLI command and flags

node zipit.js

Configuration

No response

CLI output

this is the output of the zipFunction // zip-out/index/index.mjs

` import {createRequire as nfyCreateRequire} from "module"; import {fileURLToPath as nfyFileURLToPath} from "url"; import {dirname as _nfyPathDirname} from "path"; let _filename=nfyFileURLToPath(import.meta.url); let dirname=__nfyPathDirname(nfyFileURLToPath(import.meta.url)); let require=___nfyCreateRequire(import.meta.url);

// index.js import sdk from "@aws-sdk/client-s3"; var a = () => { console.log("sdk", sdk); }; var testy_default = a; export { testy_default as default }; `

Environment

System: OS: macOS 14.2.1 CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz Memory: 773.20 MB / 32.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 18.17.0 - ~/.nvm/versions/node/v18.17.0/bin/node npm: 9.6.7 - ~/.nvm/versions/node/v18.17.0/bin/npm pnpm: 8.6.12 - ~/Library/pnpm/pnpm bun: 1.0.6 - ~/.bun/bin/bun

saikiran1043 commented 3 months ago

We see Readme.md says @aws-sdk/* is excluded on node18 and later only for zisi bundler, but it is getting excluded for esbuild as well. Is the readme incorrect (or) its the code issuePlease let us know if there is any specific reason to exclude that package in node18 and later for esbuild? Readme link: https://github.com/netlify/build/tree/e31f6da5d8d9f4dbaf70798a04d429dba72928d9/packages/zip-it-and-ship-it#zisi

esbuild/special_cases link: https://github.com/netlify/build/blob/main/packages/zip-it-and-ship-it/src/runtimes/node/bundlers/esbuild/special_cases.ts#L23

Skn0tt commented 3 months ago

zip-it-and-ship-it is built to deploy functions to AWS Lambda. Lambda includes the AWS Sdk into the Node.js runtime already, so we exclude it from the bundle. This ensure that your Functions are always using the SDK version that AWS gives you, and it reduces the bundle size. See https://docs.aws.amazon.com/lambda/latest/dg/lambda-nodejs.html for docs on this.

saikiran1043 commented 3 months ago

Hi @Skn0tt, thanks for the information. We are trying to use zip-it-and-ship-it in non AWS Lambda environment. Do you think we can have some kind of configuration to request to include @aws-sdk/* packages as well in bundle?

saikiran1043 commented 3 months ago

Hi @Skn0tt, thanks for the information. We are trying to use zip-it-and-ship-it in non AWS Lambda environment. Do you think we can have some kind of configuration to request to include @aws-sdk/* packages as well in bundle?

Hi @Skn0tt , can you please share your thoughts on the above

Skn0tt commented 3 months ago

That’s out of scope for this project right now, but i’ll let @eduardoboucas speak to this.

saikiran1043 commented 3 months ago

Thank you @Skn0tt for your prompt response and for clarifying the project's scope. I understand that the feature we requested is currently out of scope.

Would you be open to including this feature if we contribute to the implementation? The approach I am planning to take is to add an additional parameter (e.g., includeAWSSDK: true/false) to the options/config object, allowing esbuild users to include the package without any other impact.

zipFunctions(srcFolders, destFolder, options?)
eg:-
zipFunctions(srcFolders, destFolder, {
    config: {
      'includeAWSSDK': true,
    },
  });

Please let me know if this approach is acceptable and if you would be open to including this feature within the project's scope. I am happy to contribute to the implementation and submit a pull request.

saikiran1043 commented 3 months ago

Thank you @Skn0tt for your prompt response and for clarifying the project's scope. I understand that the feature we requested is currently out of scope.

Would you be open to including this feature if we contribute to the implementation? The approach I am planning to take is to add an additional parameter (e.g., includeAWSSDK: true/false) to the options/config object, allowing esbuild users to include the package without any other impact.

zipFunctions(srcFolders, destFolder, options?)
eg:-
zipFunctions(srcFolders, destFolder, {
    config: {
      'includeAWSSDK': true,
    },
  });

Please let me know if this approach is acceptable and if you would be open to including this feature within the project's scope. I am happy to contribute to the implementation and submit a pull request.

Hi @Skn0tt / @eduardoboucas , can you please share your thoughts on the above.