Simplify function type signatures #198
Merged
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
Gudahtt
force-pushed
the
simplify-types
branch
from
cba08fd
to
87c8fb0
Compare
|
|
Gudahtt
force-pushed
the
simplify-types
branch
from
87c8fb0
to
cca8a9e
Compare
rekmarks
requested changes
Sep 9, 2021
src/index.ts
Outdated
| @@ -6,7 +6,7 @@ import { intToHex, isHexString, stripHexPrefix } from 'ethjs-util'; | |||
|
|
|||
| import { padWithZeroes } from './utils'; | |||
|
|
|||
| export type TypedData = string | EIP712TypedData | EIP712TypedData[]; | |||
| export type TypedData = EIP712TypedData[]; | |||
There was a problem hiding this comment.
If TypedData is only used in / for V1, can we change its name to V1TypedData or something similar?
There was a problem hiding this comment.
We could 🤔 I had left it as-is to minimize breaking changes, but I guess we're past that mattering now.
There was a problem hiding this comment.
Updated! It's now called TypedDataV1
The type signatures of many public functions were complex and misleading. This was primarily due to the widespread use of the types `TypedData` and `TypedMessage<T>`, which where inappropriate in most of the places they were used. The `TypedMessage<T>` type is used by the `signTypedData` function when the version is 'V3' or 'V4'. All references to this type outside of V3 and V4 of `signTypedData` have been removed. This also resulted in the `T` generic parameter being removed from many functions. The `TypedData` type didn't represent the actual type signature of any function, yet it was used to represent the input data for most functions. In most cases we had no specific expectations for the input type, so `unknown` is now used instead. In the case of `V1` of `signTypedData`, the expected type was always `EIP712TypedData[]`. The `TypedData` type has been reduced to just that one case where it was useful and accurate.
Gudahtt
force-pushed
the
simplify-types
branch
2 times, most recently
from
da1bf8a
to
3be88f2
Compare
The `TypedData` type has been renamed to `TypedDataV1` to indicate that this type is only intended to represent the data being signed with `V1` of `signTypedData`. JSDoc documentation has been added for the two message types, `SignTypedDataV1Data` and `TypedMessage`.
Gudahtt
force-pushed
the
simplify-types
branch
from
3be88f2
to
2bdde6b
Compare
|
Good idea! The latest commit adds docs for both of those types. |
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.
The type signatures of many public functions were complex and misleading. This was primarily due to the widespread use of the types
TypedDataandTypedMessage<T>, which where inappropriate in most of the places they were used.The
TypedMessage<T>type is used by thesignTypedDatafunction when the version is 'V3' or 'V4'. All references to this type outside of V3 and V4 ofsignTypedDatahave been removed. This also resulted in theTgeneric parameter being removed from many functions.The
TypedDatatype didn't represent the actual type signature of any function, yet it was used to represent the input data for most functions. In most cases we had no specific expectations for the input type, sounknownis now used instead. In the case ofV1ofsignTypedData, the expected type was alwaysEIP712TypedData[]. TheTypedDatatype has been reduced to just that one case where it was useful and accurate.