feat(lambda-nodejs): esbuild bundling #11289
Conversation
|
|
…o lambda-nodejs-esbuild
Co-authored-by: Aidan Steele <[email protected]>
|
@floydspace this is now ready for review, your feedback is more than welcome, thx! |
|
|
||
| for (const mod of modules) { | ||
| if (!pkgDependencies[mod]) { | ||
| throw new Error(`Cannot extract version for ${mod} in package.json`); |
There was a problem hiding this comment.
should we also/only check version in the package-lock.json file so it would not fail in case we do not have excluding package installed in our package.json
for instance, I have aws-xray-sdk-core as my dependency and willing to bundle it, but to successfully bundle it I have to exclude the child dependency pkginfo from the bundle and push it to node_modules
There was a problem hiding this comment.
Will look into this. Maybe the right way to do this is to use npm list or yarn list and get the versions from there.
There was a problem hiding this comment.
Can you detail your use case with aws-xray-sdk-core? Why do you need to exclude pkginfo in this case?
There was a problem hiding this comment.
when I try to bundle aws-xray-sdk-core esbuild warns:
node_modules/pkginfo/lib/pkginfo.js:104:15: warning: This call to "require" will not be bundled because the argument is not a string literal
contents = require(dir + '/package.json');
so it will not be able to also bundle content from require(dir + '/package.json'); and it could lead to unpredicted behavior, so I better exclude it and put the package pkginfo with its dependencies as it is in node_modules to avoid unpredicted behavior
There was a problem hiding this comment.
due to this require condition, I also have to exclude 'import-from' for example:
node_modules/import-from/index.js:4:46: warning: This call to "require" will not be bundled because the argument is not a string literal
...ts = (fromDirectory, moduleId) => require(resolveFrom(fromDirectory, modul...
~~~~~~~
node_modules/import-from/index.js:8:9: warning: This call to "require" will not be bundled because the argument is not a string literal
return require(resolveFrom(fromDirectory, moduleId));
There was a problem hiding this comment.
these are all inherited dependencies, not my project's directly ones
There was a problem hiding this comment.
OK, got it. But why don't you set aws-xray-sdk-core as external then? Is there a big difference?
There was a problem hiding this comment.
yes, it would work, aws-xray-sdk-core is not so heavy package. But we should also take into account that it will bring also these packages along:
"continuation-local-storage": "^3.2.0",
"moment": "^2.15.2",
"pkginfo": "^0.4.0",
"semver": "^5.3.0",
"underscore": "^1.8.3",
"winston": "^2.2.0"
so it could lead to a tangible difference
There was a problem hiding this comment.
OK, I think the best solution is to parse the result of npm list <mod1> <mod2> --depth 0 --json or yarn list --pattern "<mod1>|<mod2>" --depth=0 --json.
The JSON format is different for npm and yarn.
I suggest doing this in another PR.
Can you open an issue for that? |
Co-authored-by: Elad Ben-Israel <[email protected]>
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
@eladb https://github.com/aws/aws-cdk/pull/11289/checks?check_run_id=1417379312
|
Lookup the version in the `package.json` and then fallback to requiring the module's `package.json`. This allows to mark transitive dependencies as externals and install them. See aws#11289 (comment)
Lookup the version in the `package.json` and then fallback to requiring the module's `package.json`. This allows to mark transitive dependencies as externals and install them. See #11289 (comment) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Replace Parcel with esbuild for bundling.
esbuild offers impressive performances compared to Parcel.
Moreover everything can be configured via the CLI. This means that
we don't need to play with the user
package.jsonfile anymore.Add full Windows support for local bundling.
Refactor and clean-up.
Closes #10286
Closes #9130
Closes #9312
Resolves #11222
BREAKING CHANGE: local bundling now requires
esbuildto be installed.projectRoothas been replaced bydepsLockFilePath. It should point to your dependency lock file (package-lock.jsonoryarn.lock)parcelEnvironmenthas been renamed tobundlingEnvironmentsourceMapshas been renamed tosourceMapBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license