feat: Allow to decode t as opt t #716
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report 📦
|
frederikrothenberger
changed the title
This is a strawman PR to implement coercion of `t` to `opt t` when decoding values.
frederikrothenberger
force-pushed
the
frederik/opt-error
branch
from
a58d018
to
08aa25c
Compare
| @@ -180,7 +180,9 @@ test('IDL encoding (arraybuffer)', () => { | |||
| IDL.encode([IDL.Vec(IDL.Nat8)], [new Uint8Array()]); | |||
| IDL.encode([IDL.Vec(IDL.Nat8)], [new Uint8Array(100).fill(42)]); | |||
| IDL.encode([IDL.Vec(IDL.Nat16)], [new Uint16Array(200).fill(42)]); | |||
| expect(() => IDL.encode([IDL.Vec(IDL.Int8)], [new Uint16Array(10).fill(420)])).toThrow(/Invalid vec int8 argument/); | |||
| expect(() => IDL.encode([IDL.Vec(IDL.Int8)], [new Uint16Array(10).fill(420)])).toThrow( | |||
| /Invalid vec int8 argument/, | |||
There was a problem hiding this comment.
The formatter insists on this change.
|
This is something that I've also encountered lately, if possible it would be great to merge it- |
stopak
reviewed
May 9, 2023
| public decodeValue(b: Pipe, t: Type): [T] | [] { | ||
| const opt = this.checkType(t); | ||
| if (!(opt instanceof OptClass)) { | ||
| throw new Error('Not an option type'); | ||
| return [this._type.decodeValue(b, opt)]; |
There was a problem hiding this comment.
I did some tests, and this should be a little bit different
const type = Rec()
type._type = opt
return [this._type.decodeValue(b, type)];
Otherwise this will throw another error
There was a problem hiding this comment.
Thanks. Could you provide a full test-case of an input the crashes with the current code but succeeds using your suggested change?
It would be helpful to add it to the actual test suite.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a strawman PR to implement coercion of
ttoopt twhen decoding values.The test suite still passes, but I'm not sure if I broke something else in the process.
Also it uses control flow by exception, which is rather ugly.
Fixes # (issue)
How Has This Been Tested?
Unit tests have been added to test the feature.
Checklist: