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

Better TypeScript support please. Remove any type, use generics well, and export useful types. #83

Open
ADTC opened this issue Sep 16, 2024 · 2 comments

Comments

@ADTC
Copy link

ADTC commented Sep 16, 2024

Polotno SDK seems to have subpar TypeScript support. The type system seems to be added on as an afterthought and isn't robust or always helpful. Often it hinders development because I have to fight with the confusing type system.

I hope this can be rectified by ensuring Polotno SDK has excellent TypeScript support.

I'd completely replace all instances of any with the actual types, or generics. I would also export more of the useful types instead of locking them up locally.

For example: the ImageGrid component should have generics: (the ImageType is the generic)

// Note that it shouldn't be a const but a function. Also there's no need to expand all the props.
export declare function ImagesGrid<ImageType>(props: Props<ImageType>): React.JSX.Element;

Simply doing this alone will automagically type all the props of ImageGrid correctly. If an array of specific type is passed in to images:

image

Then the callback functions like getPreview, getCredit etc. would show the correct type in the function parameter:

image

This is very helpful when writing the callback function code, as we can be type-safe in how we're accessing the incoming function parameter object.

Right now, this isn't possible because the type would just be any and the most I can do is to manually assert the correct type.

PS: There's an ESLint rule @typescript-eslint/no-explicit-any which I enable to warn me of all usages of any. It's possible to ignore it on a per-line or per-file basis if needed, but that should be seldom used.

@lavrton
Copy link
Collaborator

lavrton commented Sep 18, 2024

Please upgrade to the last version. It should work better.

@ADTC
Copy link
Author

ADTC commented Sep 18, 2024

Thank you. It works better as the callbacks in ImagesGrid are typed correctly with the images array's element type. I also appreciate that the Section type is now exported.

Hope these improvements continue and there's stronger typing where possible, reducing the usage of any to only instances where the type is truly unknown to you.


One example that stuck in my mind (and will need quite a bit of work) is the use of a generic Element type which contains the common properties for all elements, and then more specific elements like ImageElement and TextElement will take Element type as the base type and superimpose additional properties specific to them.

Then it will be possible to type store.pages.children as IArrayType<Element> instead of IArrayType<any>. (PS: The Beta Custom Elements feature will need to ensure custom elements also use Element type as their base.)

(PS: I noticed there's already ElementType in group-model but it isn't shared as I mentioned.)


Another is whether custom property can be typed as custom?: { [key: string]: any } instead custom: any.

But this would be a breaking change for devs who ignored docs and put non-compliant data in custom property (e.g. custom: "string value"). So maybe it's debatable and perhaps best avoided at this point, and announced as a breaking change in the next major or minor version.


Thank you again, for accommodating so many requests from me. 🙂

PS: One more: element in onSelect in type Props<ImageType> should have ElementType, not ShapeType.

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

No branches or pull requests

2 participants