chore(TS): object mixins #8414

ShaMan123 · 2022-10-31T21:22:11Z

Motivation

Description

This PR migrates all remaining object mixins.
I had to split files that defined both canvas and object mixins. The canvas mixins were left untouched.
I tried a different approach in this PR, using applyMixins.
My conclusion is that we should avoid it. It is fragile and TS doesn't stomach it well.
I prefer the approach of collection mixin or chaining classes.
For dynamic protos like with SVG applyMixins is fine.

Changes

Gist

As long as a mixin doesn't have a constructor or a super class applyMixins works as expected
I had to disable constructor assigning since it overrides the prev.
But I guess there is a way to merge them, I can't think of a mixin which needs it

In Action

commit f9071cf Author: ShaMan123 <[email protected]> Date: Mon Oct 31 21:52:32 2022 +0200 Update transform_files.mjs commit 6ebf5ff Author: ShaMan123 <[email protected]> Date: Mon Oct 31 20:31:18 2022 +0200 Update transform_files.mjs commit 6433c0b Author: ShaMan123 <[email protected]> Date: Mon Oct 31 20:09:43 2022 +0200 Update transform_files.mjs commit 5793489 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 20:01:55 2022 +0200 Update transform_files.mjs commit 06a12b0 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 19:39:23 2022 +0200 Update transform_files.mjs commit 4111587 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 19:37:41 2022 +0200 Create out.ts commit a2f29c0 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 18:56:48 2022 +0200 Update transform_files.mjs commit 6587d19 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 18:49:32 2022 +0200 Update transform_files.mjs commit c3bf999 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 18:45:46 2022 +0200 Update transform_files.mjs commit 8105167 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 18:37:17 2022 +0200 Update transform_files.mjs commit b8de8aa Author: ShaMan123 <[email protected]> Date: Mon Oct 31 18:34:27 2022 +0200 Update transform_files.mjs commit 2d1d1da Author: ShaMan123 <[email protected]> Date: Mon Oct 31 18:22:47 2022 +0200 Update transform_files.mjs commit 781fe55 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 18:18:01 2022 +0200 Update transform_files.mjs commit daf443f Author: ShaMan123 <[email protected]> Date: Mon Oct 31 18:16:48 2022 +0200 Update transform_files.mjs commit 3c55455 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 17:41:30 2022 +0200 Update transform_files.mjs commit f567fcf Author: ShaMan123 <[email protected]> Date: Mon Oct 31 17:24:54 2022 +0200 Update transform_files.mjs commit 9ab2ea3 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 17:16:13 2022 +0200 Update transform_files.mjs commit b3c73d1 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 17:10:30 2022 +0200 Update transform_files.mjs commit f6d9031 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 16:51:10 2022 +0200 Update transform_files.mjs commit 6208499 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 15:51:00 2022 +0200 Update transform_files.mjs commit 483379a Author: ShaMan123 <[email protected]> Date: Mon Oct 31 13:38:10 2022 +0200 static commit 316ba1a Author: ShaMan123 <[email protected]> Date: Mon Oct 31 13:28:03 2022 +0200 Update transform_files.mjs commit bcb3124 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 12:00:43 2022 +0200 Update transform_files.mjs commit 1d555ad Author: ShaMan123 <[email protected]> Date: Mon Oct 31 11:53:37 2022 +0200 Update transform_files.mjs commit 5a04341 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 11:38:11 2022 +0200 Update transform_files.mjs commit 322c271 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 10:13:37 2022 +0200 Update transform_files.mjs commit 9b7f6f9 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 09:57:55 2022 +0200 Update transform_files.mjs commit e86aa95 Author: ShaMan123 <[email protected]> Date: Mon Oct 31 08:32:27 2022 +0200 Update transform_files.mjs commit f23227f Author: ShaMan123 <[email protected]> Date: Mon Oct 31 08:26:14 2022 +0200 ast!

This reverts commit a1db53e.

github-actions · 2022-10-31T21:23:47Z

Build Stats

file / KB (diff)	bundled	minified
fabric	997.833 (-6.876)	311.408 (-0.746)

github-actions · 2022-10-31T21:25:18Z

Coverage after merging ts-object-mixins into master will be

82.70%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
HEADER.js	54%	48.15%	0%	63.64%	12, 14, 14, 14, 14, 14, 16–17, 21, 21–22, 22, 22, 24, 24, 26, 28, 30–31, 77, 77, 77
src
cache.ts	97.06%	90%	100%	100%	56
canvas.class.ts	93.36%	90.22%	94.12%	95.54%	1047, 1047–1048, 1051, 1071, 1071, 1106, 1139–1140, 1168–1169, 1202, 1210, 1320–1321, 1323–1324, 1344–1345, 1506, 1511, 1521, 1525, 474–475, 480, 489, 638–640, 685–686, 735–736, 739, 741, 784–786, 828, 833–834, 862–863
config.ts	77.27%	66.67%	66.67%	84.62%	130, 138–140, 151–153
constants.ts	100%	100%	100%	100%
intersection.class.ts	100%	100%	100%	100%
pattern.class.ts	92.19%	85.71%	100%	96.30%	109, 115, 126, 135, 87
point.class.ts	100%	100%	100%	100%
shadow.class.ts	98.51%	96%	100%	100%	156
static_canvas.class.ts	90.15%	83.90%	96.70%	92.73%	1112–1113, 1113, 1113–1114, 1248, 1314–1315, 1318, 1367–1368, 1461, 1476, 1480, 1506–1507, 1536–1537, 1570–1571, 1612–1613, 1616, 1618, 1618, 1618, 1618, 1622, 1622, 1622–1624, 1646–1647, 1688–1689, 1692, 1694, 1694, 1694, 1694, 1698, 1698, 1698–1700, 1771, 1771–1772, 1830, 1832, 1832, 1832, 1832, 1832–1833, 1836–1837, 1837, 1837–1838, 1841, 1841, 1841, 1843, 1846, 1852, 1854–1855, 1855, 1855, 1858–1859, 1859, 1859, 1862, 319–320, 322–323, 325–326, 339–340, 342–343, 57, 658–661, 861
src/brushes
base_brush.class.ts	100%	100%	100%	100%
circle_brush.class.ts	1.52%	0%	0%	2%	101, 103–105, 114, 114, 114, 116, 118, 120–122, 124–127, 135, 142, 144, 24, 29–30, 38–42, 46–50, 57–60, 68–72, 74, 82, 82, 82, 82, 82–83, 85, 85, 85–88, 90, 98–99
pattern_brush.class.ts	97.14%	87.50%	100%	100%	22
pencil_brush.class.ts	91.91%	85.42%	100%	93.64%	123–124, 153, 153–155, 277, 281, 286–287, 69–70, 85–86
spray_brush.class.ts	1.16%	0%	0%	1.56%	100–102, 104–105, 113, 113, 113, 113, 113–114, 116–117, 124–125, 127, 129–133, 142, 146–147, 147, 155, 155, 155–158, 160–163, 167–168, 170, 172–175, 178, 185–186, 188, 190–191, 193, 200–201, 203–204, 207, 207, 214, 214, 218, 23–24, 26–28, 28, 28–30, 34, 43, 50, 57, 64, 71, 78, 90–92
src/color
color.class.ts	91.67%	84.51%	100%	94.44%	325–326, 330–331, 334–335, 41, 45, 72–73, 73, 75, 75, 75–76, 78–79
color_map.ts	100%	100%	100%	100%
constants.ts	100%	100%	100%	100%
index.ts	100%	100%	100%	100%
util.ts	100%	100%	100%	100%
src/controls
actions.ts	100%	100%	100%	100%
changeWidth.ts	100%	100%	100%	100%
control.class.ts	93.98%	88.89%	90.91%	97.78%	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%	125, 132
drag.ts	100%	100%	100%	100%
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	94.41%	94.74%	100%	93.59%	129–130, 132–134, 181–183, 42
scaleSkew.ts	80.56%	66.67%	100%	92.31%	14, 28, 30, 30, 30, 32, 34
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%	83.33%	100%	90%	11–12
WebGLProbe.ts	40.54%	40%	60%	36.36%	28–30, 30, 30–31, 33–35, 43, 46–48, 48, 48–51, 53, 58
base_filter.class.ts	20.47%	20.41%	31.82%	18%	100–102, 102, 102–103, 110–112, 112, 112–113, 120–123, 123, 123–124, 130, 130, 130–133, 151, 181–186, 190–191, 191, 191–194, 194, 194, 194, 194–196, 202, 211–212, 217–221, 264–267, 276, 287–288, 288, 288–289, 291, 307–309, 309, 309, 309, 309–310, 312, 314–315, 317–318, 320–322, 330–331, 333, 337–339, 343, 343, 343, 347, 347, 347–348, 370, 370, 370–374, 402, 58–59, 81–82, 84, 84, 84–85, 85, 88, 93–95, 97, 97, 97, 97, 97, 97–98
blendcolor_filter.class.ts	6.82%	0%	25%	6.35%	101, 101, 101, 101, 101, 101, 101, 101, 101, 101, 101, 101, 103–106, 108–111, 113–116, 119–122, 124–127, 129–132, 134–137, 139–140, 140, 143–144, 144, 147–148, 148, 151, 153–156, 158–160, 175, 190–195, 60, 76, 80, 90–94, 96–99
blendimage_filter.class.ts	52.86%	70%	9.09%	59.18%	130, 138–139, 154, 170–172, 180,

src/mixins/CanvasStraighteningMixin.ts

asturur · 2022-10-31T21:37:43Z

i have a bunch of questions about the overlap of ancestry and stacking.
Why does stacking needs to extends ancestry?

Does ancestry have any usefulness outside the interactivity with activeSelection and group in general?

ShaMan123 · 2022-10-31T21:47:53Z

Why does stacking needs to extends ancestry?

That was a bad attempt I forgot to revert

ShaMan123

ready

ShaMan123 · 2022-10-31T22:45:11Z

src/shapes/fabricObject.class.ts

+
+// TODO somehow we have to make a tree-shakeable import
+
+applyMixins(InteractiveFabricObject, [


This is not bad
Though we can chain them with extends... like geometry and interactivity
Or we can make all use this pattern.

src/shapes/fabricObject.class.ts

ShaMan123 · 2022-10-31T22:46:31Z

src/util/applyMixins.ts

+) {
+  constructors.forEach((baseCtor) => {
+    Object.getOwnPropertyNames(baseCtor.prototype).forEach((name) => {
+      name !== 'constructor' &&


changed!
No constructor overrides!

ShaMan123

Conflicts have been addressed

Does ancestry have any usefulness outside the interactivity with activeSelection and group in general?

I think so. It is useful in some cases. And since Group is now a powerful object I guess more use cases will emerge and with it needs to probe the object tree.

IMO the stacking/ancestry mixins should be part of the prototype chain of FabricObject, perhaps the rest as well. SVG can be left as a dynamic mixin.
But the decision will not be breaking so we can move forward and consolidate it afterwards.
Thoughts?

ShaMan123

combined animation mixin with straightening since both animate object.
Must say that the animation mixin is a vile piece of work.
src/mixins/object_stacking.mixin.ts will be removed in #8461 , kept only for tests

ShaMan123 · 2022-11-27T17:19:13Z

src/mixins/object_animation.mixin.ts

+
+    to = to.toString();
+    if (!propIsColor) {
+      if (~to.indexOf('=')) {


What is this?
I want to simplify it but I have no idea what it does

Can/should be done in a follow up

this is the ancestor of to.includes('=') the tilde is a binary not operator, so bascially since -1
https://wsvincent.com/javascript-tilde/

how old was this line?

f270ca0#diff-58618d9e7746cd656ccf18e71d215f47c64423197bf3f0c262b941035811c8e5

More than 10 years

ShaMan123 · 2022-11-27T17:20:27Z

src/shapes/fabricObject.class.ts


 // TODO somehow we have to make a tree-shakeable import

-export { InteractiveFabricObject as FabricObject };
+export class FabricObject extends applyMixins(InteractiveFabricObject, [
+  FabricObjectSVGExportMixin,


Should we put the svg mixin in the proto chain as well?

ShaMan123 · 2022-11-27T17:21:38Z

src/mixins/object_animation.mixin.ts

+   * @param {String} to Value to animate to
+   * @param {Object} [options] Options object
+   */
+  _animate<T>(key: string, to: T, options: TAnimationOptions = {}) {


This method is so ugly
Once #8297 is merged I will rework this mixin to use an array animation perhaps... with color somehow

index.js

asturur · 2022-11-28T21:42:16Z

@ShaMan123 i m merging because nothing struck me here.
I know i have no clear how we are going to deal with mixins in general, probably we should take the time to brainstorm some idea.
Conditionally mix permutations of mixins doesn't seem an option here.

Co-authored-by: Andrea Bogazzi <[email protected]>

ShaMan123 added 10 commits October 31, 2022 22:20

init

0bb65f9

Update object_ancestry.mixin.ts

39aa829

apply mixins

c4f72d3

mixins

0333f38

split

4a87ccc

s

1e6c012

r

b83df40

imports

d252e36

Revert "Squashed commit of the following:"

eebf759

This reverts commit a1db53e.

ShaMan123 added 2 commits October 31, 2022 23:31

Merge branch 'master' into ts-object-mixins

0cf73bf

Update CHANGELOG.md

9b16b18

asturur reviewed Oct 31, 2022

View reviewed changes

src/mixins/CanvasStraighteningMixin.ts Outdated Show resolved Hide resolved

ShaMan123 added 3 commits October 31, 2022 23:38

svg mix

1985962

rename

50f14da

Update object_stacking.mixin.ts

f0e24f2

ShaMan123 added 9 commits October 31, 2022 23:48

Merge branch 'master' into ts-object-mixins

0530d84

split

537d056

Update fabricObject.class.ts

a01a55d

Update fabricObject.class.ts

c03ba7c

Update object.svg_export.ts

d23c73c

Update object.svg_export.ts

1ccd7a5

Update object_animation.mixin.ts

fc1b615

Update object_animation.mixin.ts

7ef6efa

revert

3ab5f74

ShaMan123 commented Oct 31, 2022

View reviewed changes

ShaMan123 added 6 commits November 27, 2022 12:46

Update applyMixins.ts

a972e95

try fix non applied mixin

d178930

Update index.js

39c056b

fix imports

e110457

Update object.js

c8dc7fe

fix import

3f4e2a8

ShaMan123 commented Nov 27, 2022

View reviewed changes

ShaMan123 requested a review from asturur November 27, 2022 11:47

ShaMan123 added 13 commits November 27, 2022 17:48

BREAKING: rm object stacking methods

c80efe1

merge straightening into animation mixin

efe92f7

rename

cc075ca

Update index.js

90b0dc5

Update object_animation.mixin.ts

a031d4c

Update object.class.ts

4ce9226

a bit of cleanup

e03cfed

checkout

52cf10c

Update object_animation.mixin.ts

42c826e

Update fabricObject.class.ts

7b93359

revert

8af3477

Update fabricObject.class.ts

02c360d

Update object_stacking.mixin.ts

d7433c1

ShaMan123 commented Nov 27, 2022

View reviewed changes

ShaMan123 and others added 3 commits November 27, 2022 19:42

Update object_animation.mixin.ts

27157bc

Update object_animation.mixin.ts

e6ef7a1

Merge branch 'master' into ts-object-mixins

7eebd70

asturur approved these changes Nov 28, 2022

View reviewed changes

ShaMan123 merged commit fe6d9df into master Nov 29, 2022

ShaMan123 deleted the ts-object-mixins branch November 29, 2022 07:45

frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023

chore(TS): migrate object mixins (fabricjs#8414)

27cf774

Co-authored-by: Andrea Bogazzi <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(TS): object mixins #8414

chore(TS): object mixins #8414

ShaMan123 commented Oct 31, 2022 •

edited

Loading

github-actions bot commented Oct 31, 2022 •

edited

Loading

github-actions bot commented Oct 31, 2022 •

edited

Loading

asturur commented Oct 31, 2022

ShaMan123 commented Oct 31, 2022

ShaMan123 left a comment

ShaMan123 Oct 31, 2022

ShaMan123 Oct 31, 2022

ShaMan123 left a comment •

edited

Loading

ShaMan123 left a comment

ShaMan123 Nov 27, 2022

ShaMan123 Nov 27, 2022

asturur Nov 28, 2022

ShaMan123 Dec 1, 2022

ShaMan123 Nov 27, 2022

ShaMan123 Nov 27, 2022

asturur commented Nov 28, 2022


		// TODO somehow we have to make a tree-shakeable import

		applyMixins(InteractiveFabricObject, [

chore(TS): object mixins #8414

chore(TS): object mixins #8414

Conversation

ShaMan123 commented Oct 31, 2022 • edited Loading

Motivation

Description

Changes

Gist

In Action

github-actions bot commented Oct 31, 2022 • edited Loading

github-actions bot commented Oct 31, 2022 • edited Loading

asturur commented Oct 31, 2022

ShaMan123 commented Oct 31, 2022

ShaMan123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShaMan123 left a comment • edited Loading

Choose a reason for hiding this comment

ShaMan123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asturur commented Nov 28, 2022

ShaMan123 commented Oct 31, 2022 •

edited

Loading

github-actions bot commented Oct 31, 2022 •

edited

Loading

github-actions bot commented Oct 31, 2022 •

edited

Loading

ShaMan123 left a comment •

edited

Loading