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: remove ts-ignore and ts-expect-error #245

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

EvanHahn
Copy link
Contributor

@EvanHahn EvanHahn commented Sep 3, 2024

Depends on #244.

This change should have no user impact.

There's almost always something better than // @ts-ignore and // @ts-expect-error. They can be problematic because they're very broad, and typically broader than we want. For details, see "At least 7 reasons to avoid @ts-expect-error and @ts-ignore".

This removes all of them in favor of more targeted solutions. (Or, in one case, just removes it and everything was fine!)

I think this is a useful change on its own, but it should make an upcoming change easier too.

This change should have no user impact.

There's almost always something better than `// @ts-ignore` and
`// @ts-expect-error`. They can be problematic because they're very
broad, and typically broader than we want. For details, see
["At least 7 reasons to avoid @ts-expect-error and @ts-ignore"][0].

This removes all of them in favor of more targeted solutions. (Or, in
one case, just removes it and everything was fine!)

I think this is a useful change on its own, but it should make [an
upcoming change][1] easier too.

[0]: https://shane-o.dev/articles/any-or-expect
[1]: https://github.com/digidem/mapeo-schema/pull/243
Copy link
Member

@gmaclennan gmaclennan left a comment

Choose a reason for hiding this comment

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

Looks ok with me. I'm in favour of just sticking // @ts-ignore in there when developing initial versions - sometimes jumping through hoops to get typescript passing can be a waste of dev time, especially in scenarios when TS just can't figure things out - however cleaning this up at a later stage like now is good.

@EvanHahn EvanHahn merged commit 1cd9a5a into validate-shouldnt-throw Sep 4, 2024
6 checks passed
@EvanHahn EvanHahn deleted the ts-ignore-begone branch September 4, 2024 13:13
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.

2 participants