Cleanup clinical entity data table #173
Conversation
|
@joneubank tagging for visibility as I know you were looking at this also. fixing TS build issues. |
| @@ -29,8 +29,7 @@ import { | |||
| import union from 'lodash/union'; | |||
| import { ComponentProps, ReactNode, createRef } from 'react'; | |||
|
|
|||
| export type ErrorReportColumns = { header: string; id: string }; | |||
|
|
|||
| export type ErrorReportColumns = any; | |||
There was a problem hiding this comment.
This file needs additional work. #175
Changing to any type for now to keep ErrorNotification fix isolated to it's own branch.
There was a problem hiding this comment.
This makes me incredibly nervous... Its hard to get any types out once they are in, everything that uses them will have no type checking performed.
| clinicalData: { | ||
| programShortName: '', |
There was a problem hiding this comment.
not ideal but works ok as a type-fix - this is just an empty object meant as a default value to display nothing - "emtptyClinicalDataResponse"
the root cause is the mismatch between TS types and GQL generated types.
There was a problem hiding this comment.
Where is this used, could we change this from an empty object to a function that returns the empty object?
export const emptyClinicalDataResponse =
(programShortName: string): SubmittedDataSideMenuQuery =>
({/*...content of the empty object but with a populated short name...*/});
ciaranschutte
requested review from
joneubank,
demariadaniel and
justincorrigible
| clinicalData: { | ||
| programShortName: '', |
There was a problem hiding this comment.
Where is this used, could we change this from an empty object to a function that returns the empty object?
export const emptyClinicalDataResponse =
(programShortName: string): SubmittedDataSideMenuQuery =>
({/*...content of the empty object but with a populated short name...*/});| @@ -29,8 +29,7 @@ import { | |||
| import union from 'lodash/union'; | |||
| import { ComponentProps, ReactNode, createRef } from 'react'; | |||
|
|
|||
| export type ErrorReportColumns = { header: string; id: string }; | |||
|
|
|||
| export type ErrorReportColumns = any; | |||
There was a problem hiding this comment.
This makes me incredibly nervous... Its hard to get any types out once they are in, everything that uses them will have no type checking performed.
.../(post-login)/submission/program/[shortName]/clinical-data/ClinicalEntityDataTable/index.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const { clinicalData } = clinicalEntityData; | ||
|
|
||
| // TODO: can you have ClinicalEntityErrors without ClinicalEntityData? |
There was a problem hiding this comment.
Technically yes ClinicalEntityErrors is supposed to work as a separate standalone query, so this TODO can be removed.
However I tested and it is not working in Develop. I have a patch PR coming in shortly.
|
discussed offline, adding back in ErrorNotification type. build will remain broke but will address full fix in seperate PR |
Context:
Lots of code "copy/pasted" from
platform-uihave TS types written be developers.These clash with generated types from GQL endpoints.
The GQL generated types is the correct way, as our written don't always encompass the types correctly and are prone to error.
Additionally we have many places that have empty objects as default values. These cause type errors when they differ from the actual returned object. Example, a filter function is expecting
ClinicalData[]but it actually getsEmptyClinicalData[]that doesn't quite satisfy type constraints. This leads to writing code to type narrow/account for both of these types OR making them satisfy the same type. Array methods in particular are tricky.Another difference is there is a major TS version change between
platform-uiandrdpc-uiso the "copy/paste" approach fromplatform-uisurfaced a lot of errors. ( v4 vs v5 )Changes:
This PR mostly changes types, fixes a couple of generics and adds extra property to satisfy types.
A couple of instances of using
anybecause it's a bigger issue that needs to be fixed at a component level.For clinical data page a lot of code reorganisation to solve type issues by type narrowing in correct place eg. if no data/loading, there's often a lot of code we don't ever need to worry about unless we do have data or are not loading.
Disables ESLint on build. We can re-enable later when repo is in better shape. For now step 1 is having project successfully build.