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

Missing empty string in .d.ts parameters for non-string types in attributes #1866

Closed
BalagePMI opened this issue Dec 16, 2024 · 2 comments
Closed

Comments

@BalagePMI
Copy link

The problem

The .d.ts (typescript mapper) for Shape and many other data types are not supporting string (or at least empty string constant) as input.

For example, the following declaration

 fillPatternImage: GetSet<HTMLImageElement | HTMLCanvasElement, this>;

only allows Image and Canvas types as input.
Clearing a pattern fill could be by passing the fillPatternImage an empty string (it could be undefined, but it may be some design decision), which is marked as uncompatible value. So using this one hase to add a @ts-ignore which is an antipattern and also may generates a warning itself in strict mode.

The same goes on any other field, so the code is polluted with @ts-ignore:

 // @ts-ignore
 this._node.fill('')
 // @ts-ignore
 this._node.fillPatternImage('')
 // @ts-ignore
 this._node.fillLinearGradientColorStops('')
 // @ts-ignore
 this._node.fillRadialGradientColorStops('')

The suggestion

The values which receives non string values should be extended with empty string pattern as input.

 fillPatternImage: GetSet<HTMLImageElement | HTMLCanvasElement | '', this>;

This solution has only impact on the .d.ts files.
Accepting undefined and/or null may be even cleaner. However, it has a great impact on the base JS code.

@BalagePMI BalagePMI changed the title Missing string type in .d.ts parameters Missing empty string in .d.ts parameters for non-string types in attributes Dec 16, 2024
@lavrton
Copy link
Member

lavrton commented Dec 16, 2024

I dont think empty string is a good value. While it is working in runtime because it is falsy, I think the more consistent value should be null

@BalagePMI
Copy link
Author

Yes, null or even more undefined is better, but the current definition doesn't let it either.

So my proposal is to change the

 fillPatternImage: GetSet<HTMLImageElement | HTMLCanvasElement | undefined, this>;

the signature (and for all fields).

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