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

chore(types): suppress errors with @ts-expect-error directive instead of @ts-ignore #3259

Merged
merged 7 commits into from
May 8, 2024

Conversation

satelllte
Copy link
Contributor

@satelllte satelllte commented May 8, 2024

About

Using @ts-expect-error is preferred over @ts-ignore and considered a better practice.

To see why, check out this great article by @mattpocock: How to use @ts-expect-error

Exceptions

User-facing /** @ts-ignore */ pragmas are preserved due to #3062

Copy link

codesandbox-ci bot commented May 8, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6148010:

Sandbox Source
example Configuration

@@ -7,9 +7,7 @@ import { AttachType, catalogue, Instance, InstanceProps, LocalState } from './re
import { Dpr, Renderer, RootState, Size } from './store'

// < r141 shipped vendored types https://github.com/pmndrs/react-three-fiber/issues/2501
/** @ts-ignore */
Copy link
Member

Choose a reason for hiding this comment

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

Note that any /** @ts-ignore */ is intended to be kept and are user-facing at build-time #3062. This won't be an issue in v9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@CodyJasonBennett But why? The build doesn't seem to fail because there's no error for them. Am I missing something?

Copy link
Member

Choose a reason for hiding this comment

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

Some of these types are removed in future three types, but we need to keep them here so they are dynamically used if available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, got it, I'll get them back then.
Great thing tho that v9 will have those being fully dynamic: #3038 (comment) 💎

@satelllte satelllte marked this pull request as ready for review May 8, 2024 18:53
@CodyJasonBennett CodyJasonBennett changed the title TypeScript: suppress errors with @ts-expect-error directive instead of @ts-ignore chore(types): suppress errors with @ts-expect-error directive instead of @ts-ignore May 8, 2024
@CodyJasonBennett CodyJasonBennett merged commit 9705b47 into pmndrs:master May 8, 2024
2 checks passed
@satelllte satelllte deleted the ts_expect_error branch May 9, 2024 13:18
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.

None yet

2 participants