-
-
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): minor changes to typescript notation to be compatible with a 5.3.3 #9725
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Build Stats
|
…into typedoccompatibility
@@ -602,7 +602,8 @@ export class FabricImage< | |||
ctx: CanvasRenderingContext2D | |||
) { | |||
ctx.imageSmoothingEnabled = this.imageSmoothing; | |||
// @ts-expect-error TS doesn't respect this type casting | |||
// cant use ts-expect-error because of ts 5.3 cross check | |||
// @ts-ignore TS doesn't respect this type casting | |||
super.drawCacheOnCanvas(ctx); |
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 on the called method would fix the issue without the need of silencing the error:
drawCacheOnCanvas(ctx: CanvasRenderingContext2D) {
const self = this as TCachedFabricObject;
ctx.scale(1 / self.zoomX, 1 / self.zoomY);
ctx.drawImage(
self._cacheCanvas,
-self.cacheTranslationX,
-self.cacheTranslationY
);
}
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 makes me mad that typescript has to error (not warn!!!!) on unused error directives.
So you can silence errors ( dangerous ) but you can't silence error silencers for good code.
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'm not following you. If you use ts-expect-error
then I like that it errors if there isn't actually a type error. What you're trying to do with typedoc while keeping the current TS version is not really a common use case for them.
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.
Yes i know is not common, but is still silly that i cant keep that as a warning and has to block the compilation.
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.
Does typedoc allow to specify the TS instead? So that it's aligned with fabric
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.
maybe. But this is the shortest and more logical path.
No code changes, just annotation changes.
Not fiddling with dependencies of dependencies of astro, just make it run.
I need to merge this and publish if i want to do more progress on the website. |
Description
typedoc uses TS 5.3.3 that gets angry at some our of ts-expect-error that are no more error in 5.3.3
There is now way to disable those because those are meant to inform you what to do when you migrate.
In order to make typedoc working i need to swap those with just an ignore and leave the original comment on top of it for migration.
This PR also remove the old typedoc plugin and template, but not the configuration file that is used by astro to produce the typedocs