Closed veej closed 7 years ago
@veej can you rebase on master to get the new shiny CI?
All these T
and K
are confusing... can't we rename them with a longer more meaningful name?
Ok, the build script was faulty and wasn't moving the .d.ts
files from any subfolder to the relative subfolder in lib
=> in alinity pro we didn't have the typings for Column
, Cell
etc.
Once I fixed this (and the other issue pointed by @giogonzo) I finally managed to have a file with Tablo
that typechecks and compiles 🎉
(even if the code is veeery boilerplaty...)
Just noticed that some of the remaining boilerplate can be removed pretty easily: once you have MyTable
(= Tablo<DataType, Colums>
), all the children
type parameters needed for Column
and Cell
can indeed be inferred by TS:
T
is available because of the children
typing (in both cases: Tablo.children: Column[]
and Column.children: Cell[]
)K
can be easily inferred since it is an argument: Column.name
So I think @veej your very first example in this PR description can be simplified (didn't try but you shouldn't need to pass <DataType, 'foo'>
to Column
and Cell
)
This is the resulting usage for the same AlinityPRO table: https://github.omnilab.our.buildo.io/buildo/alinity-pro/blob/dc75dec829231027c8c4a6110463e18c7587dfe1/qia/web/src/app/components/AlertCenterGrid/bsq/AlertCenterGrid.tsx#L64-L133 , which I think is acceptable.
The only thing I'm not quite getting is why the first argument of Cell.children
is not inferred correctly, e.g.:
{Column({
name: 'category',
width: columnsWidths.category || 90,
children: [
<Header key='0'><TextOverflow label={this.formatMessage('AlertCenterGrid.bsq.category')} /></Header>,
Cell({
hAlignContent: 'center',
//
// can't directly use `_` as `category` here because it is inferred as `string | number | Date | undefined`
// even weirder, IntelliSense gives me the correct `Critical | Alert | Notification` hovering on the first arg :/
// v
children: (_, { category }) => <CategoryCell category={category} />
})
]
})}
The only thing I'm not quite getting is why the first argument of Cell.children is not inferred correctly
That's the reason why I explicitly added generics to columns and cells. For some reason, inference is buggish on children, and adding explicit generics is the only way I found to be sure everything has the right type
@giogonzo, I fixed all remaining K extends keyof T
in K extends string
(defaulted to keyof T
only in component/plugins declaration).
Are we ready for merging?
Closes #1134
Test Plan
tests performed
Since
JSX
types for children are quite broken, I tried to reach a stable version at least using children as functional components:In this way a managed to have correct types even for
data
androwData
inCell
child function (withdata: undefined
in case of complex columns, where column name is not an attribute ofDataType
).Notice that, by default, you can only use simple columns (where columns' names are
keyof T
), andTablo
will complain if you use unknown colums. In case you need to use compound columns, you can redefineTablo
like this: