doc: mark Certificate methods as static, add missing KeyObject.from by panva · Pull Request #37198 · nodejs/node · GitHub
Skip to content

doc: mark Certificate methods as static, add missing KeyObject.from#37198

Closed
panva wants to merge 1 commit into
nodejs:masterfrom
panva:doc-crypto-missing-static-methods
Closed

doc: mark Certificate methods as static, add missing KeyObject.from#37198
panva wants to merge 1 commit into
nodejs:masterfrom
panva:doc-crypto-missing-static-methods

Conversation

@panva

@panva panva commented Feb 3, 2021

Copy link
Copy Markdown
Member

@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Feb 3, 2021
@panva panva force-pushed the doc-crypto-missing-static-methods branch from eae8700 to 5218bbe Compare February 3, 2021 10:12
@panva panva changed the title doc: mark crypto methods as static, add missing KeyObject.from doc: mark Certificate methods as static, add missing KeyObject.from Feb 3, 2021
@panva panva force-pushed the doc-crypto-missing-static-methods branch from 5218bbe to 9396d6d Compare February 3, 2021 10:33
@panva panva added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 3, 2021
@tniessen

tniessen commented Feb 5, 2021

Copy link
Copy Markdown
Member

@panva

panva commented Feb 5, 2021

Copy link
Copy Markdown
Member Author

@tniessen would you like to introduce a specifically named method instead and mark this one deprecated? Either way i think this doc update should land.

@tniessen

tniessen commented Feb 5, 2021

Copy link
Copy Markdown
Member

would you like to introduce a specifically named method instead and mark this one deprecated?

That depends. It works and there is no ambiguity when it comes to the input. We can reuse the same function with different signatures for other inputs.

It depends on what other input forms we want to allow in the future. What about JWK, for example? If we allow KeyObject.from(jwkObject), then we suddenly create the chance of having ambiguous inputs because JWK is a plain object and not an instance of a class that is specific to Node.js.

In the other PR (#37218), adding KeyObject.prototype.toCryptoKey(webCryptoAlgorithm, usages, extractable) was suggested. If that's going to happen, then renaming KeyObject.from to KeyObject.fromCryptoKey might make sense. I've taken the liberty of opening #37240.

If we never want to allow anything but uniquely identifiable types, such as CryptoKey, as inputs to KeyObject.from, then we might as well leave it at that. However, I'm afraid that at some point, someone will try to fit the generic API name to other inputs.

@tniessen

tniessen commented Feb 5, 2021

Copy link
Copy Markdown
Member

(Approving under the assumption that #37240 will update the docs accordingly.)

@panva

panva commented Feb 5, 2021

Copy link
Copy Markdown
Member Author

panva added a commit that referenced this pull request Feb 5, 2021
PR-URL: #37198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@panva panva closed this Feb 5, 2021
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
PR-URL: #37198
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This was referenced Feb 16, 2021
@panva panva deleted the doc-crypto-missing-static-methods branch October 13, 2022 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants