chore: switch to using JavaScript Modules (ESM) #5233
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stefreak
force-pushed
the
chore/esm
branch
2 times, most recently
from
848b5b0 to
a5476e6
Compare
stefreak
reviewed
Oct 16, 2023
stefreak
force-pushed
the
chore/esm
branch
3 times, most recently
from
b5f05be to
e55b9e7
Compare
Merged
stefreak
reviewed
Nov 2, 2023
| curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | ||
|
|
||
| # To be able to run cargo | ||
| source "$HOME/.cargo/env" |
There was a problem hiding this comment.
Optional: it's likely enough to source this once somewhere.
stefreak
reviewed
Nov 2, 2023
stefreak
reviewed
Nov 2, 2023
| "source-map-support": "^0.5.21", | ||
| "tar": "^6.2.0", | ||
| "undici": "^5.26.5", | ||
| "unzipper": "^0.10.14" |
There was a problem hiding this comment.
we can get rid of unzipper entirely by saving the whole nodejs archive to disk, and then extracting
stefreak
reviewed
Nov 2, 2023
stefreak
reviewed
Nov 2, 2023
stefreak
reviewed
Nov 2, 2023
stefreak
force-pushed
the
chore/esm
branch
4 times, most recently
from
6d32cb9 to
a44fe47
Compare
This had many different effects that derived from it, making this PR so massive. **Modules**: The first stage was to rewrite all the imports to use .js endings everywhere. Then we needed to replace and bump a bunch of dependencies to their ESM version. **Bundling**: Following that we tried to use rollup to bundle everything back into CommonJS so that we can use it in pkg since pkg does not support ESM. This was unsuccessful, and after trying several different single application solutions, we decided we will have to write our own. **Single Executable Binary**: We now use a custom Rust binary which bundles a zipped version of NodeJS, the native extensions and the bundled code. We bundle the code for tree-shaking and size reduction, but also because otherwise we had issues with how we ensure that native modules are loaded correctly and with resolving the imports correctly cross-platform. Then on first run, the binary extracts those files to the filesystem and runs node from there. There are checksum files that we bundle together with the archives and that we store on disk to see if we need to re-extract or not. This also gives us a huge performance boost in some cases - especially when lots of filesystem reads are required - since pkg was patching and proxying those methods and making things slow. **Kubernetes client**: We also had to update the Kubernetes library to the 1.0.0-rc3, with a patch from a fork that contains the changes for kubernetes-client/javascript#1341 That is because the library was using request-promise behind the scenes, which started showing up with unhandled rejection warnings and errors about req not being defined in some cases. That library update required us to change almost every k8s call since the interface now uses objects instead of positional arguments, which makes things much nicer to read and use. We also had to introduce a workaround for using custom certificates together with proxies, something that was previously globally monkey-patched by global-agent. **request-promise**: We no longer depend on request-promise and fully removed the dependency. Fixes #3841 Fixes #4898
stefreak
approved these changes
Nov 6, 2023
This was referenced Nov 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Switches to ESM.
This had many different effects that derived from it, making this PR so massive.
Changes
Modules
The first stage was to rewrite all the imports to use
.jsendings everywhere.Then we needed to replace and bump a bunch of dependencies to their
ESMversion.Bundling
Following that we tried to use
rollupto bundle everything back intoCommonJSso that we can use it inpkgsincepkgdoes not support ESM.This was unsuccessful, and after trying several different single application solutions, we decided we will have to write our own.
Single Executable Binary
We now use a custom Rust binary which bundles a zipped version of NodeJS, the native extensions and the bundled code.
We bundle the code for tree-shaking and size reduction, but also because otherwise we had issues with how we ensure that native modules are loaded correctly and with resolving the imports correctly cross-platform.
Then on first run, the binary extracts those files to the filesystem and runs node from there.
There are checksum files that we bundle together with the archives and that we store on disk to see if we need to re-extract or not.
This also gives us a huge performance boost in some cases - especially when lots of filesystem reads are required - since
pkgwas patching and proxying those methods and making things slow.Kubernetes client
We also had to update the Kubernetes library to the
1.0.0-rc3, with a patch from a fork that contains the changes for kubernetes-client/javascript#1341That is because the library was using
request-promisebehind the scenes, which started showing up with unhandled rejection warnings and errors aboutreqnot being defined in some cases.That library update required us to change almost every k8s call since the interface now uses objects instead of positional arguments, which makes things much nicer to read and use. We also had to introduce a workaround for using custom certificates together with proxies, something that was previously globally monkey-patched by
global-agent.request-promise
We no longer depend on
request-promiseand fully removed the dependency.Which issue(s) this PR fixes:
Fixes #3841
Fixes #4898
Special notes for your reviewer: