netlify/build

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

Opened this issue · 7 comments

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, }, }); };
5. 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

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

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.

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, 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

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

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.

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.