-
-
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
chore(TS): types for toObject
and additionaProperties
#8677
Conversation
Build Stats
|
As usual the important thing is that we can still export any property that has been added by the developer. |
@ShaMan123 i m not sure i can solve the conflicts correctly, the TS declarations confuses me at this point.
|
ok i tried with the conflicts. in case revert my commit |
|
||
static fromObject(object: SerializedCircleProps): Promise<Circle> { | ||
return super.fromObject(object) as Promise<Circle>; | ||
} |
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.
something i noticed in the previous PR too, i would argue this can be solved with a declaration?
is weird that TS would force me to add code that does nothing to obey types.
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.
correct
I was lazy
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.
This is another TS problem. Static class methods can't be generic using straight forward TS
padding: number; | ||
} | ||
|
||
export interface ObjectProps extends ObjectBaseProps { |
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 must have done something wrong because i don't see where this part went
@ShaMan123 please i want to move on with this, so if you have spare time to work on fabric, instead of working on other prs prioritize this one. let's understand how this should be done and then eventually i can complete it for the other classes if you don't have time. |
for today i m off |
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.
src/shapes/Object/ObjectProps.ts
turned into src/shapes/Object/types
recommended review order:
Types | Classes |
---|---|
src/shapes/Object/types/BaseProps.ts |
src/shapes/Object/ObjectOrigin.ts |
src/shapes/Object/types/SerializedObjectProps.ts , src/shapes/Object/types/ObjectProps.ts |
src/shapes/Object/Object.ts |
src/shapes/Object/types/FabricObjectProps.ts |
src/shapes/Object/InteractiveObject.ts |
src/shapes/Object/types/index.ts |
src/shapes/Object/FabricObject.ts |
src/shapes/Circle.ts |
Each type file depends on more types so review them as part of it
cornerSize: number; | ||
|
||
/** | ||
* Size of object's controlling corners when touch interaction is detected | ||
* @type Number | ||
* @default 24 | ||
*/ | ||
touchCornerSize: number; | ||
|
||
/** | ||
* When true, object's controlling corners are rendered as transparent inside (i.e. stroke instead of fill) | ||
* @type Boolean | ||
* @default true | ||
*/ | ||
transparentCorners: boolean; | ||
|
||
/** | ||
* Color of controlling corners of an object (when it's active) | ||
* @type String | ||
* @default rgb(178,204,255) | ||
*/ | ||
cornerColor: string; | ||
|
||
/** | ||
* Color of controlling corners of an object (when it's active and transparentCorners false) | ||
* @since 1.6.2 | ||
* @type String | ||
* @default null | ||
*/ | ||
cornerStrokeColor: string; | ||
|
||
/** | ||
* Specify style of control, 'rect' or 'circle' | ||
* This is deprecated. In the future there will be a standard control render | ||
* And you can swap it with one of the alternative proposed with the control api | ||
* @since 1.6.2 | ||
* @type 'rect' | 'circle' | ||
* @default rect | ||
* @deprecated | ||
*/ | ||
cornerStyle: 'rect' | 'circle'; | ||
|
||
/** | ||
* Array specifying dash pattern of an object's control (hasBorder must be true) | ||
* @since 1.6.2 | ||
* @type Array | null | ||
*/ | ||
cornerDashArray: number[] | null; |
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 must say that corner is bad naming, should be control
updated desc, ready |
toObject< | ||
T extends Omit<Props & TClassProperties<this>, keyof SProps>, | ||
K extends keyof T = never | ||
>(propertiesToInclude?: K[]): { [R in K]: T[K] } & SProps { |
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.
This thing is unreadable and who invented typescript is responsible for my eyes bleeding
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.
It was not easy to write and definately not to test
i had the laptop offline for all day because after a forced update it wasn't starting anymore ( i think the battery went off during update ). |
toObject
typestoObject
and additionaProperties
Motivation
#8674 opens up to possibility to type
toObject
Description
TS limitations of static signatures prevents me from declaring an overload to Circle.fromObject
I decided to override though there is an option to declare the lass in a .d file and change the static signature there I think.
I researched it too long.
Since we are talking about 50 classes or more it sounds better to have a .d file handle it.
Changes
A lot of types
Moved declare fields around
Gist
In Action
Try the following, test autocomplete in new, toObject, fromObject
index.copy.tsx.-.fabTest.-.Visual.Studio.Code.2023-02-08.14-23-05_Trim.mp4