Stricter types #1077
Merged
Stricter types #1077
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nextstrain-bot
temporarily deployed
to
nextstrain-s-victorlin--3p4dgg
2 tasks
victorlin
commented
Nov 15, 2024
victorlin
force-pushed
the
victorlin/type-narrowing
branch
from
3e161f8
to
f8feb3e
Compare
victorlin
force-pushed
the
victorlin/stricter-types
branch
from
2da26c3
to
becb43d
Compare
genehack
approved these changes
Nov 19, 2024
static-site/src/components/ListResources/IndividualResource.tsx
Outdated
Show resolved
Hide resolved
I originally wanted `no-non-null-assertion`, then realized that it was part of the strict ruleset. All violations of this ruleset seem reasonable to address, which I will do in subsequent commits. <https://typescript-eslint.io/users/configs/#strict>
This makes it appropriate for other components outside of ListResources to re-use this functionality.
With the current code, it is not possible for these values to be undefined. Make this clear with type narrowing.
Instead of checking null-ness to initialize an empty array then using a non-null assertion which is loosely coupled to the preceding line, do both in one line.
With the current code, it is not possible for this to be undefined. Make this clear with type narrowing.
ESLint has no way of knowing that this is safe code. Add an exception for this line with a comment explaining what it does.
The parent constructor is automatically used by default. <https://typescript-eslint.io/rules/no-useless-constructor/>
This should not be used unless there is no other way for the TypeScript compiler to infer the type properly. In such cases, a comment with per-line exception should be used. I will address existing violations in subsequent commits.
This properly handles any new changes to input values defined by the form.
I remember this was tricky and couldn't figure out a way to map featured-analyses.yaml to SplashTile[] without this type assertion.
The type is already set on this variable.
I'm not sure if it's possible for a non-Error error to be thrown in these parts, but TypeScript thinks it is possible and it's easy to handle.
victorlin
force-pushed
the
victorlin/stricter-types
branch
from
becb43d
to
e62d01c
Compare
This better reflects actual usage.
genehack
approved these changes
Nov 20, 2024
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
While working on #1073, I thought it was reasonable to avoid casting and non-null assertions across the entire codebase. This is an attempt to enforce it and address outstanding violations.
Related
Follow-up to #1073 (comment)
Checklist