Return expected data when using Promises with crypto.generateKeyPair #13600
Conversation
src/js/internal/util/inspect.js
Outdated
| @@ -133,6 133,16 @@ const { | |||
| Uint8Array, | |||
| } = primordials; | |||
|
|
|||
| const kCustomPromisifyArgsSymbol = Symbol('customPromisifyArgs'); | |||
There was a problem hiding this comment.
This util/inspect.js file is 92 KB of JS
Please move this function into a separate file, so that it can be lazily loaded without loading all of util/inspect.js
There was a problem hiding this comment.
edit: it looks like we are already loading this file in util.js
so it's probably fine
There was a problem hiding this comment.
I think it was worth refactoring anyway, let me know if you disagree and I can revert/amend it.
|
|
||
| const callbackArgs = original[kCustomPromisifyArgsSymbol]; | ||
|
|
||
| function fn(...originalArgs) { |
There was a problem hiding this comment.
instead of checking for callbackArgs on every call
this should return a different function which handles the callbackArgs and a function which does not handle them
There was a problem hiding this comment.
though i see that this implementation wasn't changed much from before
There was a problem hiding this comment.
Yeah this whole commit is just moving code around, no change in behavior.
|
lets merge once it builds successfully |
Did CI just give up mid-build? They all stopped there. It builds fine for me locally. |
What does this PR do?
It enables the use of
util.promisifywithcrypto.generateKeyPairby properly returning the expected data. Previously, we'd only ever handle the first argument of the callback, hence why the public key would always get returned on its own.There are a couple other functions that could use the same work in other parts of the node library, but that can be left for follow-up work.
How did you verify your code works?
bun-debug test test-file-name.test)Fixes #9469.