lib: runtime deprecate calling Cipheriv(...) and Decipheriv(...) without new#57268
lib: runtime deprecate calling Cipheriv(...) and Decipheriv(...) without new#57268jasnell wants to merge 3 commits intonodejs:mainfrom
Cipheriv(...) and Decipheriv(...) without new#57268Conversation
Cipher was removed from the public API a while ago but we were still
defining it and exporting `require('crypto').Cipher` as `undefined`.
Silly us.
jasnell
added
crypto
|
Review requested:
|
nodejs-github-bot
added
doc
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57268 +/- ##
==========================================
- Coverage 90.24% 90.24% -0.01%
==========================================
Files 630 630
Lines 184908 184883 -25
Branches 36181 36175 -6
==========================================
- Hits 166874 166848 -26
+ Misses 11061 11055 -6
- Partials 6973 6980 +7
|
| ### DEP0190: Instantiating `node:crypto` `Cipheriv` and `Decipheriv` classes without `new` | ||
|
|
||
| <!-- YAML | ||
| changes: | ||
| - version: REPLACEME | ||
| pr-url: https://github.com/nodejs/node/pull/57268 | ||
| description: Runtime deprecation. | ||
| --> | ||
|
|
||
| Type: Runtime | ||
|
|
||
| Instantiating the [`Cipheriv`][] and [`Decipheriv`][] classes exported by the `node:crypto` | ||
| module is deprecated. |
There was a problem hiding this comment.
Can you do a first PR that doc-deprecates it, so that can be backported to existing release lines?
There was a problem hiding this comment.
I've been unable to find any examples anywhere of anyone actually calling these (at least in any open source code). I'm not sure there's an actual value in backporting a doc-only deprecation to older lines.
There was a problem hiding this comment.
It simplifies backporting of other deprecations, as it's otherwise likely we would get conflicts. I'm happy to open the doc-only PR myself if you prefer
There was a problem hiding this comment.
@aduh95 ... let's open those as separate backport PRs after landing this is main.
There was a problem hiding this comment.
Why not follow the usual process? That'd be much less work
5 participants
The first commit here is from #57266 which is expected to land first. This PR is about the second commit... deprecating calling
Cipheriv(...)andDecipheriv(...)without thenewkeyword. Technically we should probably also deprecate creating these directly even with thenewkeyword since we tell people to usecreateCipheriv(...)andcreateDecipheriv(...)to create these (both of which simply defer to callingnew ...but still).I did do a github code search trying to find any examples of folks calling these directly without the new keyword and there were zero hits suggesting that going straight to a runtime deprecation is quite safe.
/cc @nodejs/tsc