-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Intelligent constructor #217
Conversation
On-board with the |
packages/types/src/Text.ts
Outdated
} else if (input instanceof Uint8Array) { | ||
const [offset, length] = Compact.decode(input); | ||
return u8aToUtf8(input.subarray(offset, offset + length.toNumber())); | ||
} else if (Array.isArray(input)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would propose removing Array until needed. As it stands we don't have anything coming in anymore as [...]
packages/types/src/codec/Struct.ts
Outdated
: value[key] | ||
); | ||
if (value instanceof Uint8Array) { | ||
const [compactLength, typeLength] = Compact.decode(value.subarray(currentIndex)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The length needs to be decoded by the child itself, not this one.
packages/types/src/codec/Struct.ts
Outdated
@@ -43,30 +46,54 @@ export default class Struct < | |||
}, {} as E); | |||
} | |||
|
|||
static decode <S, V, T> (Types: S, value: V | Array<any>, isTuple: boolean): T { | |||
static decode<S, V, T> (Types: S, value: V | Array<any> | AnyU8a, isTuple: boolean): T { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Erk, the isTuple is a hold-over, I don't believe is is used anymore, Tuples do their own thing. (It was pre-changes when stuff still came in as Array<number>
)
bb0e0e1
to
9dd683a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, will pull it and see if all hell breaks loose :)
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Effort to remove all
fromJSON
andfromU8a
from types, and initialize them solely via constructor. Their constructor should be intelligent enough to handle all kinds of inputs (incl. of course JSON and Uint8Array inputs).For consistency, I propose that this decoding/input handling be done in the static method
decode
. The constructor will just call thisdecode
method.Related #161.
Done in this PR: