Skip to content
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

fix(TS): EventSpec recognition #8497

Merged
merged 10 commits into from
Dec 8, 2022
Merged

fix(TS): EventSpec recognition #8497

merged 10 commits into from
Dec 8, 2022

Conversation

ShaMan123
Copy link
Contributor

Motivation

Description

This PR fixes the recognition of the EventSpec generic type that was lost by Group.
Emphasizes the for #8394
Disclaimer: TS has taken a toll from me in this inane effort. Nothing good to say about it today.

Changes

Gist

In Action

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

Build Stats

file / KB (diff) bundled minified
fabric 984.138 (+0.128) 310.094 (-0.010)

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consider making FabricObjectSVGExportMixin a mixer function like collection and drop applyMixins

I need this merged

cacheCanvasEl: HTMLCanvasElement;
protected _isCurrentlyDrawing: boolean;
freeDrawingBrush: BaseBrush;
_activeObject: FabricObject;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of there belong to canvas along with drawControls and _applyCanvasStyle

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So how this passed before withtout the ts-nocheck on top?
How did i get a clea build without those types?

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

Coverage after merging fix-collection-base2 into master will be

82.77%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
HEADER.js54%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.ts97.06%90%100%100%56
   canvas.class.ts93%89.93%94.12%95.06%1047, 1047–1048, 1051, 1071, 1071, 1106, 1139–1140, 1168–1169, 1202, 1210, 1320–1321, 1323–1324, 1344–1345, 1511, 1516, 1526, 1526, 1526–1527, 1530, 474–475, 480, 489, 638–640, 685–686, 735–736, 739, 741, 784–786, 828, 833–834, 862–863
   config.ts77.27%66.67%66.67%84.62%130, 138–140, 151–153
   constants.ts100%100%100%100%
   intersection.class.ts100%100%100%100%
   pattern.class.ts92.19%85.71%100%96.30%118, 124, 135, 144, 96
   point.class.ts100%100%100%100%
   shadow.class.ts98.51%96%100%100%157
   static_canvas.class.ts92.24%86.75%96.88%94.64%1213–1214, 1214, 1214–1215, 1335, 1345, 1399–1400, 1403, 1438–1439, 1517, 1526, 1531, 1646, 1648, 1650, 1650, 1650, 1650, 1653, 1653, 1653–1654, 1706–1708, 1710, 1712, 1712, 1712, 1712, 1716, 1716, 1716–1718, 1789, 1789–1790, 1848, 1850–1851, 1851, 1851, 1854–1855, 1855, 1855, 306, 328–332, 348, 418–419, 421–422, 431, 435–436, 438–439, 957
src/brushes
   base_brush.class.ts100%100%100%100%
   circle_brush.class.ts1.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.ts97.14%87.50%100%100%22
   pencil_brush.class.ts91.91%85.42%100%93.64%123–124, 153, 153–155, 277, 281, 286–287, 69–70, 85–86
   spray_brush.class.ts1.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.ts91.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.ts100%100%100%100%
   constants.ts100%100%100%100%
   index.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   actions.ts100%100%100%100%
   changeWidth.ts100%100%100%100%
   control.class.ts93.98%88.89%90.91%97.78%235, 319, 319, 354
   controls.render.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   default_controls.ts86.67%66.67%100%100%125, 132
   drag.ts100%100%100%100%
   rotate.ts20%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.ts93.41%92.68%100%93.59%129–130, 132–134, 148–149, 181–183, 42
   scaleSkew.ts80.56%66.67%100%92.31%14, 28, 30, 30, 30, 32, 34
   skew.ts91.03%79.31%100%97.67%130–131, 162–163, 170, 176, 178
   util.ts100%100%100%100%
   wrapWithFireEvent.ts100%100%100%100%
   wrapWithFixedAnchor.ts100%100%100%100%
src/filters
   2d_backend.class.ts92%83.33%100%93.75%35–36
   FilterBackend.ts88.89%83.33%100%90%11–12
   WebGLProbe.ts40.54%40%60%36.36%28–30, 30, 30–31, 33–35, 43, 46–48, 48, 48–51, 53, 58
   base_filter.class.ts20.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.ts6.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.ts52.86%70%9.09%59.18%130, 138–139, 154, 170–172, 180, 182, 182, 197–198, 46, 50, 54–58, 62, 72–74
   blur_filter.class.ts2.17%0%0%2.90%100–101, 103–107, 120, 135–136,

@asturur
Copy link
Member

asturur commented Dec 8, 2022

Well i think there is a balance between everyhing magically typed and type safety.
Right now you lean too much on everything typed and that is tiring

@asturur asturur merged commit 3b2b140 into master Dec 8, 2022
@ShaMan123
Copy link
Contributor Author

It was because of the return type of collection mixin that was any

@ShaMan123 ShaMan123 deleted the fix-collection-base2 branch December 9, 2022 04:24
frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023
ShaMan123 added a commit that referenced this pull request Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants