[SQL Lab] Fix proptypes warning #7691
Conversation
| @@ -308,7 +308,7 @@ class SqlEditor extends React.PureComponent { | |||
| </div> | |||
| <SouthPane | |||
| editorQueries={this.props.editorQueries} | |||
| latestQueryId={this.props.latestQuery ? this.props.latestQuery.id : 0} | |||
| latestQueryId={this.props.latestQuery && this.props.latestQuery.id} | |||
There was a problem hiding this comment.
for brand new sql lab (no any query before, or user clears localStorage), the latestQuery object will be null. Is latestQueryId assigned false? To fix this, i think should change 0 be any string like '' or 'none' ?
There was a problem hiding this comment.
null && something_invalid resolves to null, so I think this is fine. It makes more sense for latestQueryId to be null if no query exists instead of something like empty string (i think)
There was a problem hiding this comment.
null is object type? The expected type is string.
There was a problem hiding this comment.
The expected type is string, not required, so null is fine right? I wasn't seeing any prop type errors anymore so i think it's ok
There was a problem hiding this comment.
oh just realized latestQueryId is optional. LGTM thanks!!
| @@ -308,7 +308,7 @@ class SqlEditor extends React.PureComponent { | |||
| </div> | |||
| <SouthPane | |||
| editorQueries={this.props.editorQueries} | |||
| latestQueryId={this.props.latestQuery ? this.props.latestQuery.id : 0} | |||
| latestQueryId={this.props.latestQuery && this.props.latestQuery.id} | |||
There was a problem hiding this comment.
oh just realized latestQueryId is optional. LGTM thanks!!
mistercrunch
added
🏷️ bot
CATEGORY
Choose one
SUMMARY
Cleans up this console warning:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
CI
ADDITIONAL INFORMATION
REVIEWERS
@graceguo-supercat