-
-
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
BREAKING: extend
, clone
=> internal cloneDeep
#8600
Conversation
patch 6cbf2a8
Build Stats
|
extend
, clone
=> internal cloneDeep
extend
, clone
=> internal cloneDeep
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 suggest viewing the diff w/o whitespace due to src/canvas/canvas_gestures.mixin.ts
ready
src/mixins/canvas_animation.mixin.ts
Outdated
import { getEnv } from '../env'; | ||
|
||
fabric.util.object.extend( | ||
Object.assign( |
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 wonder if this works, it should I guess
Aren't tests skipped?
extend
, clone
=> internal cloneDeep
extend
, clone
=> internal cloneDeep
in my opinion this pr should look like this: #8613 And we can address the other refactors of gestures and types separately. |
Motivation
closes #8599
Description
As described in the issue:
extend is an artifact of the past, no need with spread arrays.
Changes
replace all
clone(obj, true)
withcloneDeep
that uses json stringify (Not a huge fan but seems ok, especially since cloning FabricObject instance this way usestoObject
which is great) and remove allextend
,clone
calls in favor of object spreads.I would argue we can remove the need of
cloneDeep
entirely.It is used in:
Object#fromObject
: need to inspect if it is needed or a redundant safeguard because the constructor is in charge of safeguarding against mutation of propsBREAKING:
clone
andextend
are used in all examples unfortunately so the community must have adopted them.Devs using
extend
on classes should mutate the prototype directly (or withdefineProperty
) or subclass.Using
clone
orextend
to assign to an object was always a bad idea. Use lodash or whatever.Gist
In Action