Preserve zero input in Node API#2591
Conversation
|
|
| constructor(dishOrInput=null, type = null) { | ||
| this.value = new ArrayBuffer(0); | ||
| this.type = Dish.ARRAY_BUFFER; | ||
| const hasInput = Boolean(dishOrInput) || dishOrInput === 0; |
There was a problem hiding this comment.
@vetrovk Is there a reason for this approach instead of the input === undefined || input === null recommended in the issue?
| */ | ||
| function ensureIsDish(input) { | ||
| if (!input) { | ||
| if (!input && input !== 0) { |
There was a problem hiding this comment.
@vetrovk Is there a reason for this approach instead of the input === undefined || input === null recommended in the issue?
| assert(result instanceof NodeDish); | ||
| }), | ||
|
|
||
| it("should preserve numeric zero input", () => { |
There was a problem hiding this comment.
I wouldn't use "preserve" for the title of this test - I think "process" would make more sense here?
| assert.strictEqual(result.toString(), "ONXW2ZJANFXHA5LU"); | ||
| }), | ||
|
|
||
| it("chef.bake: should preserve numeric zero input", async () => { |
There was a problem hiding this comment.
I wouldn't use "preserve" for the title of this test - I think "process" would make more sense here?
| assert.strictEqual(result.toString(), "7"); | ||
| }), | ||
|
|
||
| it("ADD: preserves numeric zero input", () => { |
There was a problem hiding this comment.
I wouldn't use "preserve" for the title of this test - I think "process" would make more sense here?
|
Hi @C85297, thanks again for the review. I believe the requested changes have been addressed in the follow-up commit:
The remaining review threads now appear to be outdated. Could you please take another look when you have a chance? Thanks! |
3 participants
Fixes #2590.
This change preserves numeric
0as valid input in the Node API instead of treating it as missing input.Changes:
0in Node API input normalization0inDishconstructionchef.toHex(0),chef.bake(0, ...),chef.ADD(0, ...), andnew Dish(0)falseValidation:
env PATH="/opt/homebrew/opt/node@24/bin:$PATH" npm cienv PATH="/opt/homebrew/opt/node@24/bin:$PATH" node --no-warnings --no-deprecation --openssl-legacy-provider tests/node/index.mjsenv PATH="/opt/homebrew/opt/node@24/bin:$PATH" npm run testnodeconsumerenv PATH="/opt/homebrew/opt/node@24/bin:$PATH" npm run lintgit diff --checkNotes:
v24.17.0and npm11.13.0.