-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
chore(): Remove over-protective cloneDeep from fromObject #9621
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
src/gradient/Gradient.ts
Outdated
color: colorStop.color, | ||
offset: colorStop.offset, | ||
opacity: colorStop.opacity, | ||
})); |
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.
Gradient should have a fromObject like Pattern
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.
Build Stats
|
(enlivedMap) => { | ||
const allOptions = { ...options, ...enlivedMap }; | ||
return enlivenObjectEnlivables<any>(serializedObjectOptions, options).then( | ||
(enlivedObjectOptions) => { |
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.
enlivedObjectOptions is a shallow clone of object so we can delete or reassign properties freely.
{ extraParam, ...options }: Abortable & { extraParam?: string } = {} | ||
): Promise<S> { | ||
return enlivenObjectEnlivables<any>(cloneDeep(object), options).then( | ||
(enlivedMap) => { | ||
const allOptions = { ...options, ...enlivedMap }; |
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 only other valuable here should be signal. not worth a full spread.
Description
We need to protect from unwanted references when restoring from data.
Today we deeply clone every object, and that is expensive ( measured on an app with 91 canvases it went from 480 to around 420 ms ) and overly protective.
In the case of Object._fromObject
options
( renamed inserializedOptions
) is passing through the enliving process that already a shallow cloneOther deep level cloning are left to the single objects duty or single properties ( gradients, pattern, array ) but they need to be carefully inspected
This PR is a wip and will add more checks on data cloning later