crypto: expose Web Crypto API under crypto/web

[dnalborczyk]

this PR makes the Web Crypto API easier accessible under crypto/web, similar to fs/promises and also consistent with stream/web. I think it's also better and consistent with the naming of crypto when node.js makes it globally accessible, otherwise I think it would add confusion.

import{webcrypto}from'crypto'webcrypto===globalThis.crypto// rather confusing :/

// previouslyconst{ webcrypto }=require('crypto')const{ webcrypto }=require('node:crypto')import{webcrypto}from'crypto'import{webcrypto}from'node:crypto'// now:const{ crypto }=require('crypto/web')const{ crypto }=require('node:crypto/web')import{crypto}from'crypto/web'import{crypto}from'node:crypto/web'

currently still missing:

• doc changes
• some import tests

Regarding the current docs, the Web Crypto API docs don't show examples for cjs and esm as the other modules do. I'm wondering if the examples for cjs should really use the destructuring syntax, as the functionality is sitting on the prototype of crypto (subtle, randomUUID, getRandomValues).

I took the liberty to change the docs from:

const{ subtle }=require('crypto').webcrypto;

to:

const{ crypto }=require('crypto/web');crypto.subtle.xxx....

as opposed to:

const{ subtle }=require('crypto/web').crypto;// or:const{crypto: { subtle }}=require('crypto/web');

[Mesteery]

Personally I think it should be exported as crypto/webcrypto or crypto/web if the webcrypto property is renamed to web (to be consistent between require('crypto').web and crypto/web), related: #40197.

[dnalborczyk]

@Mesteery

Personally I think it should be exported as crypto/webcrypto or crypto/web

crypto/web would be consistent with stream/web. crypto/webcrypto is not.

if the webcrypto property is renamed to web (to be consistent between require('crypto').web and crypto/web), related: #40197.

require('crypto').web would be indeed consistent with require('crypto/web'), but it would still clash with the naming when crypto is exposed globally one day. in the distant future, people could just remove their import statement, and the code would just keep working. currently, it would be only possible if imported like import { webcrypto as crypto } from 'crypto'. It also adds additional confusion with the naming when someone is importing a thing named web (which could really be anything, whereas crypto is expressive). In addition, there is no web export on stream either.

I personally would probably just deprecate the webcrypto export, and since it's experimental, it should possibly be removed entirely when (or before) declared stable.

[Mesteery]

By the way, why not export directly the content of webcrypto in crypto/web ?
Simply module.exports = require('internal/crypto/webcrypto').crypto.

Currently, this is not really practical in my opinion in ESM in addition to not being consistent with other builtins exports.

import{crypto}from'crypto/web',const{ ... }=crypto;

In addition, there is no web export on stream either.

Yes but most builtins export their submodules (util.types, util.strict, etc.)

[Mesteery]

Oups, sorry

[dnalborczyk]

By the way, why not export directly the content of webcrypto in crypto/web ? Simply module.exports = require('internal/crypto/webcrypto').crypto.

Currently, this is not really practical in my opinion in ESM in addition to not being consistent with other builtins exports.

import{crypto}from'crypto/web',const{ ... }=crypto;

if you mean why not using a "default" export, it would make it infeasible for exporting any additional functionality under that name if needed. webcrypto is not a namespace, it's an instance of a class Crypto.

if you mean the getter exportwebcrypto is currently exported the same way, I believe it should not be included (yet?) in the node.js snapshot: for ref: #40378

In addition, there is no web export on stream either.

Yes but most builtins export their submodules (util.types, util.strict, etc.)

the difference is that the util module is exporting functions, the "submodules" on crypto are prototype methods/props, they wouldn't work with ES6 named imports. they only work when destructured, hence I mentioned in the initial issue to ideally also change the docs for cjs require, as it is misleading and potentially can cause issues.

[tniessen]

cc @jasnell

[jasnell]

I would more expect it to directly export the crypto object...

import{subtle,randomUUID}from'crypto/web'

[dnalborczyk]

my initial thought was the same, but I thought it would make it harder to export any additional (future) functionality under this export since it's a web api with any potential future additions.

named imports such as import { subtle, randomUUID } from 'crypto/web' would (should) not work, as those methods sit on the prototype of crypto, not on the module namespace.

[panva]

As a developer I do not have the expectation of finding Web APIs in ${module}/web and so I feel this addition doesn't do much.

This also rolls back on the use of import() function in ESM examples previously discussed and added ¹.

I think we should be thinking of advancing WebCrypto forward in terms of

• deciding whether to keep all the proprietary algs or not ² (done)
• fixing or documenting (non)-conformance ³ (done)
• graduating from experimental status ⁴
• exposing crypto as a global for universal javascript libraries to use ⁵

Footnotes

1. https://github.com/nodejs/node/pull/37594#issuecomment-794021447↩

2. https://github.com/nodejs/node/pull/37969#issuecomment-810019310↩

3. #39959↩

4. #37969↩

5. https://github.com/nodejs/node/pull/37969#issuecomment-809732176↩

[aduh95]

I'd prefer to expose it as a global, like it is on browsers. However, until this happens, import { crypto } from 'node:crypto/web' is indeed an improvement.

import{crypto}from'crypto/web',const{ ... }=crypto;

In general web APIs don't let you destructure them and would throw ERR_INVALID_THIS if you try to do that. Using a named import seems appropriate in this case.

[panva]

I'd prefer to expose it as a global, like it is on browsers.

Not just browsers, WinterCG compatible runtimes as well. Anyway, I agree.

However, until this happens, import { crypto } from 'node:crypto/web' is indeed an improvement.

What would be the improvement over this which works already?

import{webcryptoascrypto}from'node:crypto';

[panva]

With lib: enable global WebCrypto by default#42083 merged I believe this could be closed.