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: add experimental warning to crypto.webcrypto #35857

Closed

Conversation

panva
Copy link
Member

@panva panva commented Oct 28, 2020

This adds a once-per-process ExperimentalWarning when Web Crypto API is accessed.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Oct 28, 2020
@panva panva force-pushed the add-webcrypto-experimental-warning branch from c46a50b to 7dd99de Compare October 28, 2020 20:27
lib/crypto.js Outdated Show resolved Hide resolved
@panva panva force-pushed the add-webcrypto-experimental-warning branch 2 times, most recently from 75c71f4 to b54d29f Compare October 28, 2020 20:48
@panva
Copy link
Member Author

panva commented Oct 28, 2020

meh, i'm getting the experimental warning even by just requiring crypto, not when accessing webcrypto...

--trace-warnings

(node:22870) ExperimentalWarning: Web Crypto API is an experimental feature. This feature could change at any time
    at emitExperimentalWarning (node:internal/util:183:11)
    at node:internal/crypto/webcrypto:6:1
    at NativeModule.compileForInternalLoader (node:internal/bootstrap/loaders:278:7)
    at nativeModuleRequire (node:internal/bootstrap/loaders:307:14)
    at lazyRequire (node:internal/crypto/util:63:36)
    at Object.get [as webcrypto] (node:crypto:285:20)
    at get (<anonymous>)
    at getOwn (node:internal/bootstrap/loaders:152:5)
    at NativeModule.syncExports (node:internal/bootstrap/loaders:260:31)
    at ModuleWrap.<anonymous> (node:internal/bootstrap/loaders:240:22)

seems to be a sideproduct of ESM's syncExports? Not sure how to proceed from here.

@panva panva marked this pull request as draft October 28, 2020 21:15
This adds a once-per-process ExperimentalWarning when Web Crypto API
is accessed.
@panva panva force-pushed the add-webcrypto-experimental-warning branch from b54d29f to 34583ea Compare October 28, 2020 21:27
@panva panva mentioned this pull request Oct 29, 2020
3 tasks
@panva
Copy link
Member Author

panva commented Oct 29, 2020

@guybedford I don't want to pollute the other PR we engaged in. So here I'd like to make the crypto.webcrypto getter emit an ExperimentalWarning. Making the property non-enumerable means it won't be importable in ESM modules and pushing the warning to a subproperty of webcrypto doesn't feel right. As-is the warning is emitted even when just requiring 'crypto'.

Any other ideas?

@panva
Copy link
Member Author

panva commented Nov 24, 2020

Closing for clear lack of interest.

@panva panva closed this Nov 24, 2020
@panva panva deleted the add-webcrypto-experimental-warning 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
crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants