WIP: Typescript

[rossknudsen]

Hi @gregnb @gabrielliwerant

This is a PR mainly for discussion, rather than a request to merge code. The reasoning is due to #109 and my personal desire to have strong type support.

I've done a bit of work, as you will see, in converting the existing codebase to TS without trying to change any logic. Adding types did expose some issues, where I made changes with what seemed the correct choice to me (but it is quite likely that I've made errors here). In other places, I have just added flags for the TS compiler to ignore the issue.

I have tried the conversion a number of times and found that because the filenames have to change to .tsx that it is quite hard to sync with code changes from master, which is why I wanted to create this PR and discuss your appetite for changing to TS. Let me know your thoughts.

[rossknudsen]

Here was an example where there were two definitions of classes which the TS compiler picked up (note the same issue further down in this file). I have combined the two classes, although this may have unforeseen consequences as I don't know how JS handled the issue previously. Perhaps only the latter or former class was applied.

[tevonsb]

@rossknudsen I would also be interested in having this library have typescript support as well. We've just pulled it in to our admin panel rewrite which is all typescript.

I haven't contributed to open source previously but would be interested in starting, so let me know if I could be helpful!

[rossknudsen]

@tevonsb I'm completing my final university paper finishing in a couple of months. Until then I don't have much spare time to write code. I'm happy for you to continue working on what I've started, however it may have to be a hard fork, since the mui-datatable team would prefer it to be kept in JS.

I have some ideas on what I'd like to do, the primary one being creating a datatable that is independent of how it is rendered that would allow it to be skinned in different ways but have a rich and robust state/data API. And take the best of this API and other datatable libraries (e.g. react-table).

[gabrielliwerant]

I've discussed it elsewhere, but so it's here as well, I'm fine with adding ts definition files that live side-by-side with js, but not a conversion away from js into ts.

[patorjk]

I have combined the two classes, although this may have unforeseen consequences as I don't know how JS handled the issue previously. Perhaps only the latter or former class was applied.

I should probably be doing other things on a Friday night, but I was intrigued by this statement since I'd read today that browsers use the first definition and ignore any later duplicate definitions (I took a shot at implementing setCellHeaderProps). If you open a new tab and enter this into the console you can see that is correct:

document.getElementsByTagName('body')[0].innerHTML = '<div style="color:green" style="color:blue">hi</div>';

However, if you define a prop twice in JSX - regardless of if it's on a component or on a standard HTML tag, React always uses the last definition. You can try it here.

So for the code above, whenever there's a duplication like that, the earlier definitions should be removed. I'm not sure why React operates in the opposite way of browsers in this case, but it may come down to how JSX is turned into JavaScript code.

As far as TypeScript goes, from the devs I've talked to, some feel like it gets in the way and adds needless complexity, and some love it. I'm not sure I'm convinced it's worth the work for what it provides. It did catch a bug here, but this bug could have also been caught by adding this line to the "rules" section of the ".eslintrc" file:

"react/jsx-no-duplicate-props": "warn",

No TS needed to find this. (side note: adding this line shows that there are 3 cases of duplicate props, all in the TableHeadCell.js file)