BREAKING: `extend`, `clone` => internal `cloneDeep` #8600

ShaMan123 · 2023-01-15T06:53:30Z

Motivation

Description

As described in the issue:

clone is a ~~shitty~~ outdated and unmaintanable piece of logic. In case of cyclic objects it will break We must remove it for good. No real reason to use it any longer

extend is an artifact of the past, no need with spread arrays.

Changes

replace all clone(obj, true) with cloneDeep that uses json stringify (Not a huge fan but seems ok, especially since cloning FabricObject instance this way uses toObject which is great) and remove all extend, clone calls in favor of object spreads.

I would argue we can remove the need of cloneDeep entirely.
It is used in:

stateful: that thing is dying
Object#fromObject: need to inspect if it is needed or a redundant safeguard because the constructor is in charge of safeguarding against mutation of props
text style: easy to clone that since it has a known structure

BREAKING:
clone and extend are used in all examples unfortunately so the community must have adopted them.
Devs using extend on classes should mutate the prototype directly (or with defineProperty) or subclass.
Using clone or extend to assign to an object was always a bad idea. Use lodash or whatever.

Gist

In Action

patch 6cbf2a8

github-actions · 2023-01-15T06:55:00Z

Build Stats

file / KB (diff)	bundled	minified
fabric	938.872 (-2.097)	296.104 (-0.322)

github-actions · 2023-01-15T06:56:05Z

Coverage after merging clone-deep into master will be

82.99%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
index.ts	100%	100%	100%	100%
src
cache.ts	96.97%	90%	100%	100%	56
config.ts	77.27%	66.67%	66.67%	84.62%	130, 138–140, 151–153
constants.ts	100%	100%	100%	100%
env.ts	72.73%	53.33%	100%	79.17%	22, 22–23, 23, 23, 25, 25, 27, 29, 31–32, 62
intersection.class.ts	100%	100%	100%	100%
pattern.class.ts	93.85%	90.32%	100%	96.15%	100, 124, 135, 144
point.class.ts	100%	100%	100%	100%
shadow.class.ts	98.48%	96%	100%	100%	156
typedefs.ts	100%	100%	100%	100%
src/brushes
base_brush.class.ts	100%	100%	100%	100%
circle_brush.class.ts	0%	0%	0%	0%	100, 102–104, 113, 113, 113, 115, 117, 119–121, 123–126, 134, 141, 143, 28–29, 37–41, 45–49, 56–59, 67–71, 73, 81, 81, 81, 81, 81–82, 84, 84, 84–87, 89, 97–98
pattern_brush.class.ts	97.06%	87.50%	100%	100%	21
pencil_brush.class.ts	91.81%	85.42%	100%	93.52%	122–123, 152, 152–154, 276, 280, 285–286, 68–69, 84–85
spray_brush.class.ts	0%	0%	0%	0%	100–101, 103–104, 112, 112, 112, 112, 112–113, 115–116, 123–124, 126, 128–132, 141, 145–146, 146, 154, 154, 154–157, 159–162, 166–167, 169, 171–174, 177, 184–185, 187, 189–190, 192, 199–200, 202–203, 206, 206, 213, 213, 217, 22–23, 25–27, 27, 27–29, 33, 42, 49, 56, 63, 70, 89–91, 99
src/canvas
TextEditingManager.ts	100%	100%	100%	100%
canvas.class.ts	93.75%	90.49%	94.12%	96.09%	1143, 1143–1144, 1147, 1167, 1167, 1226, 1276–1277, 1298, 1306, 1419–1420, 1422–1423, 1443–1444, 562–563, 568, 578, 711–712, 714–715, 715, 715, 761–762, 823–824, 877–879, 909, 914–915, 944–945
canvas_events.ts	78.92%	76.85%	83.33%	79.69%	1012–1013, 1013, 1013–1015, 1017–1018, 1018, 1018, 1020, 1028, 1028, 1028–1030, 1030, 1030, 1036–1037, 1045–1046, 1046, 1046–1047, 1052, 1054, 1085–1087, 1090–1091, 1095–1096, 1213–1215, 1218–1219, 1292, 1412, 1506–1507, 1513, 1517–1518, 1534, 1556, 1603, 1608, 1656, 174, 199, 308–309, 312–316, 321, 344–345, 350–355, 375, 375, 375–376, 376, 376–377, 385, 390–391, 391, 391–392, 394, 403, 409–410, 410, 410, 453, 461, 465, 465, 465–466, 468, 550–551, 551, 551–552, 558, 558, 558–560, 580, 582, 582, 582–583, 583, 583, 586, 586, 586–587, 590, 599–600, 602–603, 605, 605–606, 608–609, 621–622, 622, 622–623, 625–629, 635, 642, 682, 682, 682, 684, 686–690, 696, 702, 702, 702–703, 705, 708, 713, 726, 753, 753, 811–812, 812, 812–813, 815, 818–819, 819, 819–820, 822–823, 826, 826–828, 831–832, 902, 914, 921, 942, 974, 995–996
static_canvas.class.ts	94.85%	89.68%	97.96%	97.19%	1086–1087, 1087, 1087–1088, 1208, 1218, 1272–1273, 1276, 1311–1312, 1389, 1398, 1398, 1402, 1402, 1449–1450, 1667, 1667–1668, 1717, 1720, 1723, 1723, 1723, 1726, 1729, 1729, 1729, 285–286, 383–384, 386–387, 846
src/color
color.class.ts	92.16%	86.49%	100%	94.29%	330–331, 335–336, 339–340, 58, 88–89, 89, 91, 91, 91–92, 94–95
color_map.ts	100%	100%	100%	100%
constants.ts	100%	100%	100%	100%
util.ts	100%	100%	100%	100%
src/controls
changeWidth.ts	100%	100%	100%	100%
control.class.ts	93.90%	88.89%	90.91%	97.73%	235, 319, 319, 354
controls.render.ts	81.63%	78%	100%	84.78%	106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
default_controls.ts	86.67%	66.67%	100%	100%	122, 129
drag.ts	100%	100%	100%	100%
polyControl.ts	6.35%	0%	0%	11.43%	100, 105, 119, 121–124, 124, 127, 134, 17, 25–28, 30, 30, 30, 30, 30, 30, 30, 30, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–83, 83, 83–84, 88–90, 90, 90, 90, 90, 92
rotate.ts	20%	12.50%	50%	22.22%	45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73, 73–74, 76, 78, 80–81
scale.ts	93.41%	92.68%	100%	93.59%	129–130, 132–134, 148–149, 181–183, 42
scaleSkew.ts	78.79%	64.29%	100%	85.71%	27, 29, 29, 29, 31, 33, 35
skew.ts	91.03%	79.31%	100%	97.67%	130–131, 162–163, 170, 176, 178
util.ts	100%	100%	100%	100%
wrapWithFireEvent.ts	100%	100%	100%	100%
wrapWithFixedAnchor.ts	100%	100%	100%	100%
src/filters
2d_backend.class.ts	92%	83.33%	100%	93.75%	35–36
FilterBackend.ts	88.89%	88.89%	100%	85.71%	15–16
WebGLProbe.ts	37.14%	40%	60%	30%	28–30, 30, 30–31, 33–35, 43, 46–48, 48, 48–51, 53, 58
base_filter.class.ts	19.88%	20.83%	30%	17.35%	100–102, 102,

f4913f6

ShaMan123

I suggest viewing the diff w/o whitespace due to src/canvas/canvas_gestures.mixin.ts

ready

ShaMan123 · 2023-01-15T07:08:34Z

src/mixins/canvas_animation.mixin.ts

-import { getEnv } from '../env';
-
-fabric.util.object.extend(
+Object.assign(


I wonder if this works, it should I guess
Aren't tests skipped?

asturur · 2023-01-19T12:47:05Z

in my opinion this pr should look like this: #8613

And we can address the other refactors of gestures and types separately.

ShaMan123 added 13 commits January 15, 2023 08:31

Update .gitignore

27d33ee

artifact mixins

f4913f6

Update stateful.mixin.ts

787efad

Update line.class.ts

98ef94f

Update Object.ts

d6f7c38

cloneDeep

bd55fa9

Update textStyles.ts

1313a97

Update index.ts

0456ecb

tests

6cbf2a8

safeguard

dc08489

fix(): test

9b58dab

patch 6cbf2a8

Update textStyles.ts

138e008

safeguard

1c2b62f

ShaMan123 added task v6 labels Jan 15, 2023

ShaMan123 requested a review from asturur January 15, 2023 06:56

Update CHANGELOG.md

11ec711

ShaMan123 changed the title ~~chore(): extend, clone => internal cloneDeep~~ BRAEKING: extend, clone => internal cloneDeep Jan 15, 2023

ShaMan123 added 3 commits January 15, 2023 09:09

Update textStyles.ts

369255d

types

98497ef

artifact imports

b20aac1

f4913f6

ShaMan123 commented Jan 15, 2023

View reviewed changes

ShaMan123 changed the title ~~BRAEKING: extend, clone => internal cloneDeep~~ BREAKING: extend, clone => internal cloneDeep Jan 15, 2023

ShaMan123 added 2 commits January 16, 2023 16:02

Merge branch 'master' into clone-deep

adcc318

this will cause a conflict with #8598

f28e66c

ShaMan123 mentioned this pull request Jan 16, 2023

refactor(IText): extract draggable text logic to a delegate #8598

Merged

Merge branch 'master' into clone-deep

9707637

ShaMan123 and others added 8 commits January 22, 2023 08:28

Merge branch 'master' into clone-deep

4eb0a37

Update DraggableTextDelegate.ts

775b1c1

Update itext_click_behaviour.js

4d3d2b5

Merge branch 'master' into clone-deep

63f99c9

Update CHANGELOG.md

dad3cbe

import reordering

6056fde

more import reordering

cf2c42d

more reordering and somewhing wrong with my master

2c9dc2b

asturur approved these changes Jan 22, 2023

View reviewed changes

asturur merged commit 2615b84 into master Jan 22, 2023

ShaMan123 deleted the clone-deep branch February 5, 2023 05:16

ShaMan123 added a commit that referenced this pull request Mar 16, 2024

BREAKING: extend, clone => internal cloneDeep (#8600)

3e9fc23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BREAKING: `extend`, `clone` => internal `cloneDeep` #8600

BREAKING: `extend`, `clone` => internal `cloneDeep` #8600

ShaMan123 commented Jan 15, 2023 •

edited by asturur

Loading

github-actions bot commented Jan 15, 2023 •

edited

Loading

github-actions bot commented Jan 15, 2023 •

edited

Loading

ShaMan123 left a comment

ShaMan123 Jan 15, 2023

asturur commented Jan 19, 2023

BREAKING: extend, clone => internal cloneDeep #8600

BREAKING: extend, clone => internal cloneDeep #8600

Conversation

ShaMan123 commented Jan 15, 2023 • edited by asturur Loading

Motivation

Description

Changes

Gist

In Action

github-actions bot commented Jan 15, 2023 • edited Loading

github-actions bot commented Jan 15, 2023 • edited Loading

ShaMan123 left a comment

Choose a reason for hiding this comment

ShaMan123 Jan 15, 2023

Choose a reason for hiding this comment

asturur commented Jan 19, 2023

BREAKING: `extend`, `clone` => internal `cloneDeep` #8600

BREAKING: `extend`, `clone` => internal `cloneDeep` #8600

ShaMan123 commented Jan 15, 2023 •

edited by asturur

Loading

github-actions bot commented Jan 15, 2023 •

edited

Loading

github-actions bot commented Jan 15, 2023 •

edited

Loading