Conversation
| if (message && typeof message !== 'string') { | ||
| throw new Error('Must specify string message.'); | ||
| } |
There was a problem hiding this comment.
Behavioral change 1 of 2:
It was previously possible to pass a truthy, non-string value as the message to error getters when using the object signature.
| import { errorCodes } from './error-constants'; | ||
|
|
||
| export { | ||
| errorCodes, |
There was a problem hiding this comment.
Behavioral change 2 of 2:
ERROR_CODES renamed to errorCodes.
There was a problem hiding this comment.
Does this require the version bump to be a breaking change?
There was a problem hiding this comment.
Yes! IMO, the first behavioral change arguably does too, even though it's a bug fix.
From the description:
A major version bump of this package will follow.
brad-decker
left a comment
brad-decker
left a comment
There was a problem hiding this comment.
A few suggestions to make this awesome work 1/10th of a step more awesome.
| import { errorCodes } from './error-constants'; | ||
|
|
||
| export { | ||
| errorCodes, |
There was a problem hiding this comment.
Does this require the version bump to be a breaking change?
| @@ -0,0 +1,108 @@ | |||
| import safeStringify from 'fast-safe-stringify'; | |||
|
|
|||
| export interface SerializedEthereumRpcError { | |||
There was a problem hiding this comment.
You should probably extends Error here to get the native types for an Error object. This will require some changes (stack is string | undefined, name is required)
There was a problem hiding this comment.
I intentionally didn't extend Error for these types, because they're intended to follow the JSON-RPC spec, not the native JavaScript error.
2 participants
Migrates this package to TypeScript. In trying to fix the types for this package, which were always terrible and never worked out of the box, I just decided to go ahead with the TypeScript migration.
Bundles two behavioral changes:
ERROR_CODESexport renamed toerrorCodesmessagefor error gettersA major version bump of this package will follow.
Fixes #7