feat(): Replacement for getKlass utility #8500

asturur · 2022-12-09T09:33:43Z

Motivation

fabric.util.getKlass required all the objects to be imported in a fabric global variable in order to be restored from json.
This made impossible to split the code you are not using with imports.
This small system of registering class when imported allow for building apps that require just some parts of fabricJS without importing the whole bundle.

The feature is written just with this in mind and can be extended later with more nuances ( as @ShaMan123 custom resolvers for some classes #8282 )

Description

Changes

Gist

In Action

github-actions · 2022-12-09T09:41:08Z

Build Stats

file / KB (diff)	bundled	minified
fabric	963.967 (+1.619)	307.933 (+0.337)

asturur · 2022-12-09T10:05:28Z

Ok i have some issues with Pattern that i didn't account for ( not understand why not for Gradient then... ) i ll fix it asap.
The extra 300bytes of code for this are mostly for all the explict calls we do, i don't think i can avoid it. ( 40 calls to .setClass can't be really minified )

ShaMan123

I don't see a real difference between #8282 to this PR.
Over there I had more freedom to solve things like Gradient not having a type and I did more work to adapt fabric not to import classes, but instead call the registry (such as in enlivenObjectEnlivables and in the elements_parser)

ShaMan123 · 2022-12-10T07:28:22Z

src/filters/blur_filter.class.ts

 } from './typedefs';
+import { isWebGLPipelineState } from './typedefs';


You can do:

import { type TWebGLPipelineState, type T2DPipelineState, type TWebGLUniformLocationMap, isWebGLPipelineState, } from './typedefs';

ShaMan123 · 2022-12-10T07:33:25Z

src/shapes/circle.class.ts

@@ -220,4 +221,7 @@ export const circleDefaultValues: Partial<TClassProperties<Circle>> = {

 Object.assign(Circle.prototype, circleDefaultValues);

+classRegistry.setClass(Circle);
+classRegistry.setSVGClass(Circle);


Why 2 calls?
Why not setClass(Circle, { svg?: boolean, key?: string })

Or better, why not check if fromElement exists...
#8282

ShaMan123 · 2022-12-10T07:34:07Z

src/shapes/circle.class.ts

@@ -220,4 +221,7 @@ export const circleDefaultValues: Partial<TClassProperties<Circle>> = {

 Object.assign(Circle.prototype, circleDefaultValues);

+classRegistry.setClass(Circle);
+classRegistry.setSVGClass(Circle);
+
 fabric.Circle = Circle;


Shouldn't we remove fabric assignments?

yes but now would make all tests fail, no?

not if we adjust the index file to use export, then rollup should build the fabric object by itself if I am not mistaken.

ShaMan123 · 2022-12-10T07:36:18Z

src/shapes/object.class.ts

@@ -934,7 +935,7 @@ export class FabricObject<
   * @param {Object} object
   */
  _removeDefaultValues(object: Record<string, any>) {
-    const prototype = fabric.util.getKlass(object.type).prototype;
+    const prototype = classRegistry.getClass(object.type).prototype;


What about registering a class and it's default values in the registry?
I understand you don't want it in this PR.

asturur · 2022-12-10T09:16:48Z

i remember #8282 was huge. Let me look again

asturur · 2022-12-10T09:28:57Z

Is hard to re-understand #8282 since now is showing diffs from an old situation, but i remember that the registry was resolving fromObject and fromElement instead that resolving the class and ther were changes to how we do fromObject and fromSVG to accomodate that. I just want to do only leaner/necessary changes now

ShaMan123 · 2022-12-10T09:56:55Z

#8282 had a large diff for the same reason this PR does, because each file must be registered. That is it.
Regarding what the registry returns, that is not a big deal IMO, though I think it makes it much more flexible.
But it is not the reason I pointed it out.
The registry itself is built in a way that svg tags or json types are easier to handle, meaning edge cases like Gradient is a one line fix. And is built to allow the dev easy overriding.

ShaMan123 · 2022-12-10T10:02:38Z

take a look at src/Registry.ts
Someone could do registry.resolveJSONKey = (...args) => 'whatever'

I would remove the assert methods and throw in the getters.
My concept was to eliminate the need for a class to have fromObject fromElement, making things a lot more flexible. But that can wait. You can see that on top of the flexible methods I've exposed a registerClass method which is equivalent to calling setClass setSVGClass of this PR.
The diff is in the possibility of fine tuning the resolver.

github-actions · 2022-12-10T10:27:05Z

Coverage after merging smaller-registry into master will be

82.81%

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	92.53%	88.10%	94.12%	95.48%	1147, 1147–1148, 1151, 1171, 1171, 1233, 1269–1270, 1289–1290, 1298–1299, 1320, 1328, 1441–1442, 1444–1445, 1465–1466, 1628–1629, 1633, 506, 573–574, 579, 589, 718–719, 721–722, 722, 722, 769–770, 831–832, 835, 838, 884–886, 916, 921–922, 951–952
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.31%	85.71%	100%	96.43%	120, 126, 137, 146, 98
point.class.ts	100%	100%	100%	100%
shadow.class.ts	98.51%	96%	100%	100%	157
static_canvas.class.ts	92.38%	85.82%	97.92%	95.16%	1118–1119, 1119, 1119–1120, 1240, 1250, 1304–1305, 1308, 1343–1344, 1422, 1431, 1436, 1551, 1553, 1555, 1555, 1555, 1555, 1558, 1558, 1558–1559, 1611–1613, 1615, 1617, 1617, 1617, 1617, 1621, 1621, 1621–1623, 1694, 1694–1695, 1750, 1752–1753, 1753, 1753, 1756–1757, 1757, 1757, 301, 333, 346, 402–403, 405–406, 415, 419–420, 422–423, 878
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	93.41%	92.68%	100%	93.59%	129–130, 132–134, 148–149, 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	7.87%	0%	25%	7.81%	100, 102, 102, 102, 102, 102, 102, 102, 102, 102, 102, 102, 102, 104–107, 109–112, 114–117, 120–123, 125–128, 130–133, 135–138, 140–141, 141, 144–145, 145, 148–149, 149, 152, 154–157, 159–161, 176, 191–196, 61, 77, 81, 91–95, 97–99
blendimage_filter.class.ts	53.52%	70%	9.09%	60%	131, 139–140, 155, 171–173, 181, 183, 183, 198–199, 47, 51, 55–59, 63, 73–75
blur_filter.class.ts	3.23%	0%	0%	4.29%	100–102, 104–108,

asturur added 6 commits December 6, 2022 17:56

some changes

751800c

merge conflicts

310a84f

some little work

6d298f4

save work

736ab63

removed namespace option

539cd8b

forgot file

5d13e1c

asturur added 3 commits December 9, 2022 10:44

fixed class registry

9243ffb

added specific test file

679ef2c

removed camelize test

7dcddba

asturur marked this pull request as ready for review December 9, 2022 10:02

no dist

00416f5

ShaMan123 reviewed Dec 10, 2022

View reviewed changes

made more similar to old getKlass

2e0f1fd

asturur and others added 3 commits December 10, 2022 11:27

changelog and prettier

dbe1077

Merge branch 'master' into smaller-registry

1d5dbbb

refactor(): image fromObject

dc0cc0f

ShaMan123 mentioned this pull request Dec 12, 2022

feat(): I/O Registry #8282

Closed

6 tasks

Merge branch 'master' into smaller-registry

00a3a55

ShaMan123 mentioned this pull request Dec 12, 2022

port Registry I/O #8507

Closed

asturur added 4 commits December 28, 2022 16:52

merged conflicts

95b6fd8

fixed small diff

2a6e185

restore resizeFilter

6e92782

fix prettier

d13f011

asturur merged commit 3fbbf83 into master Dec 28, 2022

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

feat(): Replacement for getKlass utility (fabricjs#8500)

11cd376

ShaMan123 deleted the smaller-registry branch February 6, 2023 07:05

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

feat(): Replacement for getKlass utility (#8500)

3da78fc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(): Replacement for getKlass utility #8500

feat(): Replacement for getKlass utility #8500

asturur commented Dec 9, 2022 •

edited by ShaMan123

Loading

github-actions bot commented Dec 9, 2022 •

edited

Loading

asturur commented Dec 9, 2022

ShaMan123 left a comment

ShaMan123 Dec 10, 2022

ShaMan123 Dec 10, 2022 •

edited

Loading

ShaMan123 Dec 10, 2022

ShaMan123 Dec 10, 2022

asturur Dec 10, 2022

ShaMan123 Dec 10, 2022

ShaMan123 Dec 10, 2022

asturur commented Dec 10, 2022

asturur commented Dec 10, 2022

ShaMan123 commented Dec 10, 2022 •

edited

Loading

ShaMan123 commented Dec 10, 2022

github-actions bot commented Dec 10, 2022 •

edited

Loading

		} from './typedefs';
		import { isWebGLPipelineState } from './typedefs';

feat(): Replacement for getKlass utility #8500

feat(): Replacement for getKlass utility #8500

Conversation

asturur commented Dec 9, 2022 • edited by ShaMan123 Loading

Motivation

Description

Changes

Gist

In Action

github-actions bot commented Dec 9, 2022 • edited Loading

asturur commented Dec 9, 2022

ShaMan123 left a comment

Choose a reason for hiding this comment

ShaMan123 Dec 10, 2022

Choose a reason for hiding this comment

ShaMan123 Dec 10, 2022 • edited Loading

Choose a reason for hiding this comment

ShaMan123 Dec 10, 2022

Choose a reason for hiding this comment

ShaMan123 Dec 10, 2022

Choose a reason for hiding this comment

asturur Dec 10, 2022

Choose a reason for hiding this comment

ShaMan123 Dec 10, 2022

Choose a reason for hiding this comment

ShaMan123 Dec 10, 2022

Choose a reason for hiding this comment

asturur commented Dec 10, 2022

asturur commented Dec 10, 2022

ShaMan123 commented Dec 10, 2022 • edited Loading

ShaMan123 commented Dec 10, 2022

github-actions bot commented Dec 10, 2022 • edited Loading

asturur commented Dec 9, 2022 •

edited by ShaMan123

Loading

github-actions bot commented Dec 9, 2022 •

edited

Loading

ShaMan123 Dec 10, 2022 •

edited

Loading

ShaMan123 commented Dec 10, 2022 •

edited

Loading

github-actions bot commented Dec 10, 2022 •

edited

Loading