refactor(internal-plugin-encryption): pkijs v3 + node 20 engines#4951
refactor(internal-plugin-encryption): pkijs v3 + node 20 engines#4951Tiuipuv wants to merge 7 commits into
Conversation
update pkijs to v3, and fix call points remove asn1js (using new pkijs features) remove valid-url (use native URL constructor) remove isomorphic-webcrypto (use new pkijs features) remove safe-buffer (use native Buffer constructor) remove uuid (use native uuid v4 via global crypto) fix jsdocs
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
mkesavan13
left a comment
There was a problem hiding this comment.
Could you please remove all changes to mandate node v20 or is it that the package that you import supports only above node 20? Kindly clarify. Also, please update the branch. It has conflicts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 513979b5aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…20 bumps - Replace all crypto.randomUUID() with uuid.v4() - Re-add uuid@^3.3.2 to package dependencies - Reverse package.json engines.node bumps
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d16ea4fbc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e4cdd0618
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
💡 Codex Review
ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@mkesavan13 package is up to date with latest from next. The reason for the bump to v20 was because of crypto not being exposed globally until v19 (effectively v20 for LTS) of NodeJS. However, I reverted back to the very old uuid v3 lib so we can reduce scope for the PR. Its worth noting that some of the sub packages are already requiring v20 in engines (@webex/contact-center), so they really probably should be synced across the repo up to v20 for clarity on what the effective minimum version is. |
This pull request addresses
Reduce the size of the dependency tree massively (which will allow consumers to remove
--omit=optionalfrom npm installs) due to the entire expo/react native tree being installed.Before, when running
npm install webex-js-sdkin a fresh directory will yield this:With this fix branch, it will yield this:
Lastly, this branch standardizes node ver to >=20 (up from ~18)
by making the following changes
This is done by removing deps in
internal-plugin-encryptionthat are only used by that package, and are replaced with built-in functionality of modern browsers + nodejs.The following deps are removed:
The following were updated:
v2.1.84tov3.4.0Also, this standardizes minimum NodeJS by addressing old
engines.nodefields stuck at v8, v14, v16, and v18, bumping up to v20. Similar to #4288Change Type
The following scenarios were tested
Ran unit tests, and attempted manual bad path injection to verify code coverage of kms.
For reviewing speed purposes, the actual logic changes occur just under
internal-plugin-encryption, with the following files:The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.