tleunen / react-mdl

React Components for Material Design Lite
https://tleunen.github.io/react-mdl/
MIT License
1.76k stars 255 forks source link

Typescript support #266

Open PavelPZ opened 8 years ago

PavelPZ commented 8 years ago

@tleunen first thanks a lot for creating react-mdl. I came from .NET and angular world and I surprisingly recognized that there is not any feasible ui framework for React so far (apart from horrible and not mobile friendly Material-UI or apart from react-bootstrap). I appreciate a lot your approach of react component wrapping up Google JS and CSS and I find it very promising.

However, I am missing the Typescript support. Typescript is not so popular in React community but its usage in JavaScipt and node.js (and angular 2) world is incredibly growing.

Are you interested in contributing react-mdl.d.ts Typescript definition file? If yes:

  1. for what r-mdl version (1.5, 2.0 or both)? Alternatively, will both versions have the same interface?
  2. How are you preparing ## Configuration table in documentation? I think that content of this table could be a good source for semi-automatic .d.ts file generation.
tleunen commented 8 years ago

I'm not a typescript user, but indeed it would be great having the typescript description file for all the typescript users out there :) Even if I'm thinking about v2 (which btw will be fully React, without the initial material.js from MDL), it's still in early development. So if you want the typescript support asap, it's better to do it against v 1.5 first ;) Ideally, both versions will have a really close interface. Most components won't change.

For the conf table in the doc, it's manually, but I like the idea of scanning the component and generating it automatically :)

PavelPZ commented 8 years ago

OK, I will try to parse 1.5's .propTypes objects.

PavelPZ commented 8 years ago

I prepared the first .d.ts version. I have problem with the functional properties signature. Some of them I recognized from your code or examples - can you check it out please? And there are some of them which I cannot recognize. Thanks.

Cannot recognize:

Check it out please:

tleunen commented 8 years ago

TableHeader.cellFormatter can take 3 params, but only the first one is really needed. The 2 others are just in some specific usecases. (String, Object, Number) which are the value in the cell, the row object, and the index of the row.

Your definition for TableHeader.sortFn is correct. And the others as well.

alexeyzimarev commented 8 years ago

All elements are expected to have intrinsic HTML attributes like style, I can see this from examples. Also, React.Props<T> is marked as obsolete. In addition, we need to have support for importing individual components. I started a repo for Typings, adding new elements continuously but only when I need them in my project. It may take about a week until I finish all types.

PavelPZ commented 8 years ago

@tleunen thanks, I modified TableHeader.cellFormatter to (val:string, row: any, rowIndex:number) => string. What is your idea concerning using this file in your project? Some of pure JavaScript repos has Typescript directory in the root. Or will it be better to wait for @alexeyzimarev version (which will be part of DefinitelyTyped project)?


@alexeyzimarev, thanks for info about your project, I removed obsolete React.Props<T>.

Comments:

interface CardTitleElement extends React.ReactElement<CardTitleProps> { }
const CardTitle: CardTitle;
tleunen commented 8 years ago

I don't mind adding a typescript definition file in this project, if one of you guys want to send a PR for that ;) As @alexeyzimarev mentioned, almost all elements can have intrinsic attributes.

PavelPZ commented 8 years ago

intrinsic attributes I added them to all components except of Table. The reason is that official react.d.ts definition file contains following definitions for some of the intrinsic attributes:

value?: string | string[];
rows?: number;

Those definitions are in conflict with the following react-mdl definitions: value: string | number for Radio and RadioGroup rows: Array<{}> for Table.

Possible solution is to keep value?: string | string[] for Radio and RadioGroup + to ommit intrinsic attributes for Table In order to have intrinsic attributes for Table too, it is necessary to rename Table's rows attribute.

alexeyzimarev commented 8 years ago

@PavelPZ as you can see, I define interfaces for both Props and for ComponentClass. This is because I find it not correct to define classes in type definitions although I realize that these are ambient references, still it actually would generate some real JS code, whilst interface definitions do not generate anything. In order to export a component type, I declare a constant that has interface as its type and export it.

This approach is used in type definitions for many popular component libraries like react-bootstrap.

Regarding the HTML attributes, all components are in fact HTML elements and normally should have all intrinsic HTML attributes available. These attributes are widely used in samples and this is where I got issues using your d.ts file.

I am not sure if machine generation is good or not. I am still working on my definitions file and I test all components with it. Adding oneOf is not a problem for me too but of course it will need changes if the source library changes.

PavelPZ commented 8 years ago

@alexeyzimarev, HTML attributes are already solved. d.ts machine generation is based on correct propTypes found in @tleunen code. I am successfully using react-mdl.d.ts in my eLearning project now (component props inheritance, mentioned in #273, is patched by hand).

tleunen commented 8 years ago

I see that typescript are available in the DefinitelyTyped repo. See https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/react-mdl/