Skip to content

Typescript support #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benjaminbours opened this issue Jul 28, 2018 · 27 comments
Open

Typescript support #109

benjaminbours opened this issue Jul 28, 2018 · 27 comments

Comments

@benjaminbours
Copy link
Contributor

Hello @gregnb,

I'm a typescript user and I wanted to know if you are interested in typescript support for this component ? Material-ui-next have a support for each component so I think it can be nice to have for the MUIDatatables too. I never did a declaration file before, but I think it can be a nice exercice for me so if you want i'm interested to write it.

@gregnb
Copy link
Owner

gregnb commented Jul 28, 2018

hey @benjaminbours thank you for offering. I think at this moment it's low on the priority list. Something that is really important is the documentation. I would love to finish the website (http://www.material-ui-datatables.com). I would love a draw to pop out when clicking the menu with codesandbox demos of all everything listed in the examples/ folder. Interested? :)

@benjaminbours
Copy link
Contributor Author

Yes why not :) I will take a look ! Are we okay that is something like this you are looking for ?
https://material-ui.com/demos/drawers/

@gregnb
Copy link
Owner

gregnb commented Jul 29, 2018

Yes absolutely! If you look at the way material-ui.com's menu functions it's just a draw as well. I would like it to look the same

@benjaminbours
Copy link
Contributor Author

Hi @gregnb
I'm really still OK to do your feature and I don’t forget it but I have a lot of jobs for this moment (for the agency where I work and for some personal projects).
I will look after a pull request ASAP.

@gregnb
Copy link
Owner

gregnb commented Aug 4, 2018

No worries @benjaminbours

@atrauzzi
Copy link

atrauzzi commented Aug 10, 2018

Definitely need to use this library in my TS project, so I'd also love to see typings added. Even just to remain consistent with MUI.

@nidheeshdas
Copy link
Contributor

I've written a dts for my project. It's simply a copy from the README file. Feel free to edit.
https://gist.github.com/nidheeshdas/ebdb07d147d7ff89e09bb44fe822ba31

@Diaver
Copy link

Diaver commented Oct 13, 2018

@G-Rath
Copy link

G-Rath commented Dec 20, 2018

@Diaver would you mind making a PR pulling your typings into the project, rather than forking it? That way it's easier to develop the package, as everyone can work off the single project (unless you're taking it in your own direction).

I would love this project to have typings for typescript! I'll have a crack at it myself, based off the work already done by @nidheeshdas & @Diaver, but I think it would be great to have it as a higher priority.

@gregnb I'd argue against it being low priority, as having strong type definitions is actually great for documentation - they serve as a definitive "this must be that", since typescript will throw errors in your face, making them a fantastic "single source of truth".

This package is looking pretty great, so I'm going to try and get it working in my project - I don't know if it'd be better to have the typings here, or on @types. While I'm confident in making typings for use in my own projects, I wouldn't say I can do so to the level of the guys over at DefinitelyTyped.

Any input from people with typescript exp would be appreciated - I'll drop my typings here when I'm done with them 😄


As a heads up, Ive found these typings, which seems to so far be the strongest of the three (sorry guys)

@Diaver
Copy link

Diaver commented Dec 20, 2018

@G-Rath
I made I lot of changes in my fork, not just TS support.

@gregnb
Copy link
Owner

gregnb commented Jan 5, 2019

If anyone wants to take the definitions file posted above and clean it up to match what is currently in the library I will gladly accept it. Hard to ignore the growth in TypeScript

@jacobsickels
Copy link

It looks like someone has added the project to DefinitelyTyped. However, with the recent update to the project exporting subcomponents, the types also need to be updated.

@dandrei
Copy link

dandrei commented Feb 27, 2019

Hi @gregnb, thank you for your great work on this.

I wonder if it's possible use mui-datatables in a more "type-safe" way. Maybe it's already available or maybe it's something that can be easily implemented.

Currently we specify the columns as such:

const columns = [
  {
    name: 'object_field',
    label: 'Object field'
  }
}

This will take the object_field field from each object in the array sent to data. However, specifying the field name as a string isn't type-safe. TypeScript has no way of knowing whether objects in data contain or don't contain the field object_field.

Question:

Is it possible perhaps, instead of having name as a required field, another option could be added? (let's call it value, required when name isn't specified), that would be used as such:

const columns = [
  {
    value: (obj) => obj.object_field,
    label: 'Object field'
  }
}

This way, we could use it with TypeScript:

value: (obj: KnownType) => obj.object_field,

...and TypeScript would check that object_field does indeed belong to KnownType. Since it's an extra option, it wouldn't interfere with the current way of doing things or JS-only users.

Would also be nice to get the original data object in customBodyRender. Use case for this is: if we want to render one column based on a value from another.

Right now we do have access to the values array in tableMeta.rowData, The caveat is: the order matters. Changing the column order requires a change in how we access tableMeta.rowData. But would be nice to be able to keep the type information if using TypeScript.

What are your thoughts? Thanks!

@rossknudsen
Copy link
Contributor

Hey all.

I really like this library and am keen to convert to Typescript. I've already started on a branch, so if you would like to review/contribute, that would be great.

@gabrielliwerant
Copy link
Collaborator

@rossknudsen Thanks for taking the initiative, however, rather than convert the library to ts, I think we should actually just allow it as an opt-in feature, so non-ts people (like myself!) can continue to use it according to their preference, much the same way that material-ui handles it: https://github.com/mui-org/material-ui/tree/master/packages/material-ui/src/AppBar.

@rossknudsen
Copy link
Contributor

@gabrielliwerant sure fair enough. I feel like I'm the opposite way around, a TS person, not JS!

I'm not sure if your comment that 'non-ts people can continue to use it' implies that TS libraries cannot be used in JS projects or not. But if you are implying that, then I just wanted to make the comment that TS projects can be consumed by JS only projects - since they are compiled down to JS in the end. Of course you're probably meaning the maintainers of said project would have to work in TS not JS. So I respect your choice in this regard.

@lookfirst
Copy link

@gabrielliwerant The way MUI does it leads to endless runtime bugs for them because they don't take advantage of the fact that Typescript has a compiler and they have this whole silly middle of the road stance.

Every time they (and you) make a change to something, you either have to have a test that does the same thing a compiler does and/or you have to thoughts and prayers things are just going to work.

You can fight sticking with JS for a long time for whatever religious reason you want to make up, or you can just realize that TS actually improves things over the long term. Let's use computers to do as much of the dirty work as possible.

Cheers man. =)

@rossknudsen
Copy link
Contributor

rossknudsen commented Aug 24, 2019

For anyone interested, I have a work-in-progress to implement a React hook for managing the state for datatables generally - as per my original comment. Its really just mapping out an ideal API until I get a chance to actually implement. Feel free to critique.

Edit:
Also, React Table may be an existing library that does what I'm intending.

@lookfirst
Copy link

lookfirst commented Aug 25, 2019

@rossknudsen I just took a look at your branch for typescript and you've done a really great first pass. It cleans up the code significantly. It should really get finished up and merged into this project. If I was the author of this project, I'd be really happy to have that contribution. Since that isn't the case, let me put this out there:

I'd be happy to work with you on a fork that goes 100% latest MUI, typescript and react hooks with no more react classes. I also want it to support the relay cursor spec. I'm really tired of seeing other data tables out there charging $1000 for this stuff and I can't believe this problem hasn't been solved yet with a nice consistent, well tested and performant piece of code.

Edit: React Table might be a good basis, but sadly it looks like zero tests and not typescript.

@rossknudsen
Copy link
Contributor

@lookfirst, sounds like a great idea. I haven't heard of the 'relay cursor spec' but I'm happy to learn - graphql is on my todo list.

The other suggestions (typescript, MUI latest, and React Hooks) are definitely my preference. As mentioned above, I have limited time commitment until the end of Oct, as I'm completing my final paper at University/College (I work full-time and study part-time). But can make time to review code and/or discuss design/architecture in the meantime.

It would probably benefit to start another project/organisation for this work. If you have any preferences in this regard let me know.

@KrzysztofMadejski
Copy link

KrzysztofMadejski commented Dec 10, 2019

I did send an update to DefinitelyTyped matching 2.12 version: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/40941/files

I hope to work on a PR importing it here soon. A work in progress is here: https://github.com/gregnb/mui-datatables/compare/master...KrzysztofMadejski:enhancement/issue-109-typescript-support?expand=1
I'm struggling with all these rollup/babel configurations though.

@dmythro
Copy link

dmythro commented Jun 11, 2020

Hi. Is there a support of v3 planned?

@patorjk
Copy link
Collaborator

patorjk commented Jun 11, 2020

@Z-AX I'm the current maintainer - I'd be open to a PR, though I myself don't use TypeScript.

@dmythro
Copy link

dmythro commented Jun 11, 2020

@patorjk MUI has full TypeScript support, as this is built on top — I'd expect TypeScript coverage as well. And there is for v2.

@patorjk
Copy link
Collaborator

patorjk commented Jun 11, 2020

@Z-AX it's all volunteer based, I'd be open to a PR. From previous issue reports, these kind of issues were solved by submitting a PR to the DefinitelyTyped library.

@thSoft
Copy link

thSoft commented Oct 8, 2020

https://www.npmjs.com/package/@types/mui-datatables seems pretty up-to-date.

@littleski
Copy link

It's not anymore since mui v5. :'(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests