Update the isLoading variable to check against undefined and null values #352
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
twojtasz
reviewed
Jun 25, 2019
| @@ -127,7 +127,7 @@ class XVIZMetricComponent extends PureComponent { | |||
| verticalGridLines, | |||
| getColor | |||
| } = this.props; | |||
| const isLoading = currentTime === null; | |||
| const isLoading = currentTime === null || currentTime === undefined; | |||
There was a problem hiding this comment.
doing currentTime == null would give the same result. http://www.ecma-international.org/ecma-262/6.0/#sec-abstract-equality-comparison
I suggest this just because I myself find it bizarre to have to check 2 invalid states everywhere. Normally strict equality is preferred, but this is the one case where abstract equality is meaningful in untyped languages.
There was a problem hiding this comment.
The linter was complaining / auto changing it to ===. I'll switch it back to == and add in a linter rule for it.
|
@twojtasz - updated the PR to include an in-line es lint disable rule for |
rastalamm
added a commit
to rastalamm/streetscape.gl
that referenced
this pull request
Jul 8, 2019
…ues (aurora-opensource#352) * Update the isLoading variable to check against undefined and null values * add eslint disabling to allow for 'non strict' equality checking for this line
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.
At the moment,
isLoadingis checking to see if there is acurrentTimein order to determine whether or not to display a metric. AscurrentTimeis declared but does not have a value (until it's passed in as a prop) it needs to be checked againstundefinedas well.