-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
fix: accounts controller type error #4331
fix: accounts controller type error #4331
Conversation
* Creates a deep clone of the given object. | ||
* This is to get around error `Type instantiation is excessively deep and possibly infinite.` | ||
* | ||
* @template O,T - The type of the object being cloned. |
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.
nit
* @template O,T - The type of the object being cloned. | |
* @template I,T - The type of the object being cloned. |
export function deepCloneDraft<I, T>(obj: I): T { | ||
return JSON.parse(JSON.stringify(obj)) as 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.
Not sure about this solution. I can foresee some problems if the data type has Map, Set, RegExp objects. And also the parse and stringify operations are quite expensive in terms of performance for large objects which is what the error suggests in this case. Lastly, stringify is a bit unpredictable in some cases.
I'd strongly suggest to use another solution for this case.
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.
- https://developer.mozilla.org/en-US/docs/Glossary/Deep_copy
- https://developer.mozilla.org/en-US/docs/Web/API/structuredClone
- https://saturncloud.io/blog/what-is-the-most-efficient-way-to-deep-clone-an-object-in-javascript/#method-3-recursive-function
Looking at some benchmarks, recursive deep cloning looks like to be the most effective one.
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.
Looks like on the Snap side, they are using this: https://www.npmjs.com/package/rfdc
So we're gonna use their deepClone
(from snaps-utils
)
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.
structuredClone was not available in node 16
Explanation
This PR is a temp fix for the type error
Type instantiation is excessively deep and possibly infinite.
when updating the state.References
Related to: MetaMask/utils#168
Changelog
@metamask/accounts-controller
Checklist