Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: simplify nodejs webcrypto #405

Merged
merged 1 commit into from May 2, 2023

Conversation

missinglink
Copy link
Contributor

does this make sense to clean up nodejs support for webcrypto?

Screenshot 2023-05-02 at 14 15 03

Screenshot 2023-05-02 at 14 15 15

@missinglink
Copy link
Contributor Author

missinglink commented May 2, 2023

The motivation for this was that I wanted to use a different function webcrypto.createDigest and it seemed silly to create a fallback function for each just to call them from nodejs

@missinglink
Copy link
Contributor Author

note: I changed a constant export const webcrypto to a variable export let webcrypto

@missinglink
Copy link
Contributor Author

missinglink commented May 2, 2023

agh worth noting that as of 19.4.0 this behaviour changed, I was using an 18.x.x release.

see: nodejs/node#42083

@missinglink
Copy link
Contributor Author

In fact, is it even required to call import('node:crypto') as everything seems to be available in globalThis?

node --version
v18.16.0

node
Welcome to Node.js v18.16.0.
Type ".help" for more information.
> globalThis.crypto.webcrypto.subtle
SubtleCrypto {}

@jwerle
Copy link
Member

jwerle commented May 2, 2023

In fact, is it even required to call import('node:crypto') as everything seems to be available in globalThis?

node --version
v18.16.0

node
Welcome to Node.js v18.16.0.
Type ".help" for more information.
> globalThis.crypto.webcrypto.subtle
SubtleCrypto {}

That is only true in a REPL, print, or eval context

Copy link
Member

@jwerle jwerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jwerle jwerle added api An issue, task, or discussion related to public runtime APIs node An issue, task, pull request, or discussion related to Node.js interop/compat labels May 2, 2023
@jwerle jwerle requested a review from chicoxyzzy May 2, 2023 13:14
@jwerle jwerle merged commit 2643590 into socketsupply:master May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api An issue, task, or discussion related to public runtime APIs node An issue, task, pull request, or discussion related to Node.js interop/compat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants