eric-briscoe commented on code in PR #20281:
URL: https://github.com/apache/superset/pull/20281#discussion_r909078344
##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -282,22 +293,22 @@ export default function DataSourcePanel({
}, [columns, datasource, metrics]);
const sortCertifiedFirst = (slice: ColumnMeta[]) =>
- slice.sort((a, b) => b.is_certified - a.is_certified);
+ slice.sort((a, b) => b?.is_certified - a?.is_certified);
const metricSlice = useMemo(
() =>
showAllMetrics
- ? lists.metrics
- : lists.metrics.slice(0, DEFAULT_MAX_METRICS_LENGTH),
- [lists.metrics, showAllMetrics],
+ ? lists?.metrics
+ : lists?.metrics?.slice?.(0, DEFAULT_MAX_METRICS_LENGTH),
Review Comment:
@eschutho I always try to write code that assumes every prop passed to a
component will be the wrong data type, null, undefined so that it is impossible
for the component to throw a runtime error.
TypeScript will only help us when we don't pass around object refs (without
explicit new object mapping each value to attribute on new object) component to
component which essentially makes us think we are doing type checking, but in
reality the object can be mutated at any point along the chain of props between
components drilling, or redux and end up in a bad state. We also have cases
where components that are not TypeScript migrated yet are in the mix.
While it is unlikely metrics would get set to something other than
undefined, null, or array, if it does get an incorrect value assigned this
would error out. Whenever there is not a performance concern - something
called repeatedly in loops, I prefer to make the code runtime bomb proof.
I am completely flexible on this approach and can dial back the `?.(` pre
function invocation checks if that is a preferred approach. Probably worth
deciding and discussing with team either way to be consistent :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
