src: remove SecureContext _external getter#20237
src: remove SecureContext _external getter#20237addaleax wants to merge 1 commit intonodejs:masterfrom
_external getter#20237Conversation
This is unused inside Node core, so nothing good can come from keeping it around.
nodejs-github-bot
added
c++
|
CITGM run? |
BridgeAR
added
the
author ready
tniessen
left a comment
tniessen
left a comment
There was a problem hiding this comment.
Any +1/-60 change involving node_crypto.cc looks good to me.
This is unused inside Node core, so nothing good can come from keeping it around. PR-URL: nodejs#20237 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
|
Landed in 03e25b6 🎉 |
This is unused inside Node core, so nothing good can come from keeping it around. PR-URL: #20237 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
|
This broke uWebsockets bindings for nodejs: https://github.com/uNetworking/uWebSockets-bindings/blob/master/nodejs/src/uws.js#L508 |
|
@dyatlov That library is messing with Node’s internals in multiple ways that are very sensitive to changes… reverting this might work as a short-term measure (and you can feel free to open a PR if you like), but: Do not expect that library to be usable in a stable way, there are many ways in which it could break unexpectedly. This is a larger issue that the library authors need to address – this might require opening a few issues here, but it should be worth the effort. |
11 participants
This is unused inside Node core, so nothing good can come from keeping it around.
(If somebody wants to label this
semver-major, I don’t really care, but it’s really just unused code in an internal API that can’t really be used in a meaningful way.)Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes