ui: add checks for values #99962
ui: add checks for values #99962
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 32 of 53 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained
| @@ -36,7 +36,7 @@ export const selectIndexStatsKeys = createSelector( | |||
| ); | |||
|
|
|||
| export const KeyToTableRequest = (key: string): TableIndexStatsRequest => { | |||
| const s = key.split("/"); | |||
| const s = key?.split("/"); | |||
There was a problem hiding this comment.
IDK if this is the right way to approach addressing these issues. How come we can't track down where we actually might be reading null or undefined in the issues linked? Now we have issues where this will case s to be null, and we're accessing s[0] right below this.
I think there's a lint rule we can introduce that will warn us if we're reading a potentially null field. I'd prefer tracking down the cause in the issues linked, addressing just those, and then introducing the lint rule and fixing the problems that is flagged by that. WDYT?
There was a problem hiding this comment.
The error message itself doesn't have enough info that allow us to know what is the exact place of the error, so it would be a guess at this point, so this is why I added on some other places too.
I can do another pass over all the code to make sure I didn't added on places that could cause an issue like the one you pointed.
We can add a lint for that, but the end result would still be adding the checks on all places. I will keep this PR focused on the check itself and look into the lint in another PR.
There was a problem hiding this comment.
Sounds good! I'd just like to avoid a world where we always use ? even unnecessarily, instead of properly typing things as nullish / not nullish and respecting that. As long as something like this does not become the norm and it's just a bandaid for the existing issues.
There was a problem hiding this comment.
Update the PR and now is only on places where is possible to be null and I also add to return empty strings and break loops if that was indeed the case
Fixes cockroachdb#99655 Fixes cockroachdb#99538 Fixes cockroachdb#99539 Add checks to usages that could cause `Cannot read properties of undefined`. Release note: None
maryliag
added
backport-22.2.x
backport-23.1.x
|
TFTR! |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 119c54d to blathers/backport-release-22.2-99962: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Fixes #99655
Fixes #99538
Fixes #99539
Add checks to usages that could cause
Cannot read properties of undefined.Release note: None