chore: convert to typescript #93
Conversation
|
I just noticed that I messed up the npm scripts 😦. I can fix that... |
|
Thanks for your contribution, don't worry about the documentation site, I'll learn from those two PRs, thanks again |
src/AutoResizer.tsx
Outdated
| * Decorator component that automatically adjusts the width and height of a single child | ||
| */ | ||
| const AutoResizer = ({ className, width, height, children, onResize }) => { | ||
| const AutoResizer: React.SFC<AutoResizerProps> = ({ className, width, height, children, onResize }) => { |
src/AutoResizer.tsx
Outdated
| /** | ||
| * Decorator component that automatically adjusts the width and height of a single child | ||
| */ | ||
| const AutoResizer = ({ className, width, height, children, onResize }) => { |
| const disableWidth = typeof width === 'number'; | ||
| const disableHeight = typeof height === 'number'; | ||
|
|
||
| if (disableWidth && disableHeight) { | ||
| return ( | ||
| <div className={className} style={{ width, height, position: 'relative' }}> | ||
| {children({ width, height })} | ||
| {children({ width: width!, height: height! })} |
There was a problem hiding this comment.
we can't use short hand here?
There was a problem hiding this comment.
its because of enabling strict null checks -- tsc thinks width and height might be null, so you have to override it. it works as expected if you put the null checks in the if statement.
if (typeof width === 'number' && typeof height === 'number') {
return (
<div className={className} style={{ width, height, position: 'relative' }}>
{children({ width, height })}
</div>
);
}
src/BaseTable.tsx
Outdated
|
|
||
| interface BaseTableState { | ||
| scrollbarSize: number; | ||
| hoveredRowKey: null; |
There was a problem hiding this comment.
rowKey could be only string | number, I think we could define one and use it in other places, like type RowKey = string | number;
src/BaseTable.tsx
Outdated
| onColumnResize: noop, | ||
| }; | ||
|
|
||
| // TODO: convert to interface |
src/AutoResizer.tsx
Outdated
| @@ -21,8 +43,8 @@ const AutoResizer = ({ className, width, height, children, onResize }) => { | |||
| <AutoSizer className={className} disableWidth={disableWidth} disableHeight={disableHeight} onResize={onResize}> | |||
| {size => | |||
| children({ | |||
| width: disableWidth ? width : size.width, | |||
| height: disableHeight ? height : size.height, | |||
| width: disableWidth ? width! : size.width, | |||
There was a problem hiding this comment.
noob question, what is ! for?
There was a problem hiding this comment.
it removes null and undefined from the type without having to actually check (new TS docs have a good example about it and strict null check quirks)
src/BaseTable.tsx
Outdated
| @@ -177,8 +480,8 @@ class BaseTable extends React.PureComponent { | |||
| * | |||
| * @param {object} offset | |||
| */ | |||
| scrollToPosition(offset) { | |||
| this._scroll = offset; | |||
| scrollToPosition(offset: { scrollLeft?: number; scrollTop: any }) { | |||
|
|
||
| type ClassNameFunc<T = any> = ((args: T) => string) | string; | ||
|
|
||
| export const Alignment = { |
There was a problem hiding this comment.
BTW, in the next version, I'm going to remove those consts in flavor of a | b | c
There was a problem hiding this comment.
👍 for a | b | c
babel doesnt have good support enums. also IMO you get better type checking and intellisense this way.
| /** | ||
| * Custom style for the column cell, including the header cells | ||
| */ | ||
| style: PropTypes.object, |
There was a problem hiding this comment.
the propTypes stuff is a little weird about stuff that isnt propTypes... tbh, havent removed them yet because I use them to keep track of the jsdoc comments when updating the typings.
Feel free to make any changes -- I'll go through the review comments later today. |
|
@sskiswani I'm going to merge this PR to move forward, we can take care of the trivials later |
|
Quite interested in using this library, especially if this PR is going to be merged. Do you think that will be today/this week, @nihgwu? |
nihgwu
changed the title
|
@paul-mcgrath merged, but this is for v2, won't be released in short term |
|
Ah, I missed that detail. Thank you. |
Cool, let me know ihow I can help! Also heads up, there's a code base that was giving me issues because |
I haven't been able to spend enough time on setting this up with the documentation site and examples, but from what I've been using it for -- this seems to have everything working fairly smoothly.
I opted to using tsdx for doing the builds, although it's fairly opinionated so it might be necessary to directly use rollup if necessary.
Let me know what you think!
EDIT: I just noticed the other PR has more robust generic types, you can close this PR if the other seems more promising