Rmwc: Typescript support

Created on 30 Nov 2017  Â·  67Comments  Â·  Source: jamesmfriedman/rmwc

Any chances to add Typescript definitions to the project?

Fixed in Upcoming Release help wanted

Most helpful comment

Hi there, FYI I've started some definitions (https://github.com/ncknt/rmwc-typed). It's very much a work in progress.

All 67 comments

@marcelowa, that would be an awesome addition. I don’t know much about typescript, but it is something but I’d be willing to do it especially if I can get some help.

Maybe there is a tool that can convert flow to TS?

I looked a bit for such a tool, but I couldn't find any (there's flowgen that does the opposite), but doing some reading it shouldn't be too hard to add this support
Do you mind if I take it?

Would love the contribution, you'd be my first contributor! Would be awesome if it could be automated, would be super hard to keep both up to date and in sync.

Unfortunately I can't find an existing tool to automate this

Thats a bummer @marcelowa. I'll leave the ticket open. I'd say someone can make external typings for it but it would be a large project and hard to keep up to date.

Hi there, FYI I've started some definitions (https://github.com/ncknt/rmwc-typed). It's very much a work in progress.

@ncknt, thanks for keeping up with this. I'd like to add a little section on Typescript in the getting started guide. Would you like to type up a brief paragraph with some code examples on how to setup typescript with RMWC?

Apologies for the delay. The project should become @types/rmwc or we could just merge the definition files (.d.ts) into rmwc. The latter is preferable to keep the definitions in sync but requires that someone maintains them. I'm not sure how much work it represents (basically definitions need to be updated when props change or new components are added.)

@jamesmfriedman what do you think?

If the Typescript compiler (tsc) or tslint was run as a preversion hook like the tests - or if the source was simply made into typescript files and compiled with babel before publish - that could help to "automatically" maintain the definitions.

Moving to TS though means porting over the type definitions and also a new build system. Currently its built on Create React App and it would suck to lose that.

I’m convinced someone needs to write a flow to typescript automatic build system.

If @jamesmfriedman doesn't want to maintain the typescript defs, it should be put in DefinitelyTyped (its the whole point of that project). You can also have tests in DefinitelyTyped to test the type defs without having to add any TS support in this project. But yeah the types would be super helpful either way.

I definitely am not qualified to support Typescript as I am a Flow guy. It would definitely be awesome to have it as an option for people though! If they can make their way to definitely typed I would love to document how to get them setup at least. I would just need someone to write me a blurb that I can add to the Readme (or PR it).

Sidenote @jamesmfriedman, if you replace react-scripts with react-script-ts, you can use CRA with typescript. No need to eject.

I think he's saying he would prefer Flow anyway. :)

For what it's worth, I was the same way until I got sick of Flow's RAM usage and lack of good editor tooling. You don't have to use the object-oriented aspects of Typescript, you can use it almost exactly like Flow, and it's nearly equivalent in static analysis these days. (You can also even use it with babel now in babel 7, no need to use its compiler.)

It's less preference and more toolchain. It was INCREDIBLY hard to put together something that would auto-document directly from the built in types. I went with Flow because React Docgen supported it and it seemed like the best way to make sure the code and docs stayed 100% in sync.

I think that's a good reason to choose your typing on; in the end you want the work to become less -- not more.

For anyone browsing this issue: https://github.com/styleguidist/react-docgen-typescript is promising but really not ready for production use.

I'm still hoping for TS bindings... Is this ded?

Not dead at all, I'm interesting in finding a first class way to support this. I was just looking at the shape of typescript definitions tonight and I'm convinced that an automated process can be built into the RMWC build to spit out the definition files. Can't promise anything in the near term, but definitely not falling off the radar.

Until the definitions are added I use a workaround (not much autocomplete after that) so that it compiles.
I use the button component so I added this line into a index.d.ts file in my project:
declare module 'rmwc/Button';

@marcelowa flow-to-typescript could be the tool you're/we're searching for: https://github.com/bcherny/flow-to-typescript/issues/4

It's straightforward to use, but unfortunately not ready yet.

159 has been fixed and has paved the way for this. Is there anyone available to help me work on this one?

Generating ts type definition files at transpile time could be a little finicky. Would you have any interest in a refactor from flow to typescript? Converting from flow to typescript isn't too painful and you've already done a lot of the hard work by defining types.

If we did that, then we’d have to find a way to create flow definitions at transpile time ;). I’ve been exploring this, it’s very doable and I’ll be doing a POC soon.

Welp, it's definitely possible! :). POC of automatically converting the FlowTyped javascript files to Typescript. Take a look at the Typescript branch if you're interested in whats going on. More updates soon!

image

WE HAVE TYPESCRIPT.
This is officially one of the hardest things I've ever done.

  • There were no existing automated tools that I found that worked
  • Flow and Typescript have enough similarities where they can be typed almost identical in most cases. And when they are not identical, consistency in the codebase allowed me to do a find and replace on regex patterns in files to make it typescript compatible.
  • .tsx files get generated at build time and are then run through tsc to create the definitions. The tsx files are then removed.
  • Doing this exercise actually improved the typings in RMWC all around, and fixed some hidden bugs
  • I was able to start a vanilla create-react-app with typescript and import / render / test all components inside of RMWC.

I'm declaring this beta

  • I don't use Typescript in my day to day, and the many variations of tsconfig made this hard for me to test. I'm sure configuration differences will expose bugs.
  • Please give it a try and report any feedback on this issue.

Live with 1.7.2.

I'll give it a try this weekend, and report back.

How are the TS files being generated? They're ignored in git, but published on npm (see https://unpkg.com/[email protected]/). If they're not directly based on stable definitions like some fully compatible Flow or JSDoc types, they should be committed to the repository so they can be improved manually.

@nickmccurdy that wouldn't work since the steps describe https://github.com/jamesmfriedman/rmwc/issues/40#issuecomment-398620128:

.tsx files get generated at build time and are then run through tsc to create the definitions. The tsx files are then removed.

Each release all manual edits would be removed.

@nickmccurdy you are correct that they are ignored. They are based completely on the stable flow definitions.

  • The library is built in flow
  • The tests fail on any flow failure
  • The build generates tsx files from the flow typed ones and then uses the tsx files to generate the declarations
  • The typescript compiler will error out if there are any Typescript errors causing the build to fail

So basically, there is a test / build safe way to generate these things without error. The things I'm unsure of are peoples different typescript configurations and how those might play, as well as conflicts with @types/react versions (should I make it a peer dependency?). I'm sure those things will be cleared up relatively quick.

I am using the onClick handler on an ListItem (as is done in /src/docs/Submenu.js). But it seems that the TS definitions do not include the onClick attribute for the ListItem. I assume it should get all of the React HTMLAttributes? Because (as I guess) you can see here https://github.com/jamesmfriedman/rmwc/blob/master/src/List/index.js#L60 that it includes all the leftover props.

Correct. All of the components in RMWC handle spread props, so all of the appropriate react ones should be passed on through ...rest. This allows you to pass anything React accepts without having it explicitly typed.

That's what is happening during run-time yes. But, in TypeScript, RMWC components prop types aren't extending prop types from HTMLInputElement. So the compiler is complaining about fields that are supported at run-time but not supported at compile type because they're missing.

E.g. Cannot add prop type on TextField

I was just doing some reading and it appears it’s a simple as adding a Union with React.HTMLAttributes. I take a look.

I think that should do it, yes :) Thanks you!

What @nichgalea reported, was what I tried to say. I just was not very clear about that. And I couldn't help on this since I have no idea how flow and the ts conversion process works.
And yes as far as I could see you have to extend the type of the props with: SomeRMWCPropsType & React.HTMLAttributes

Can we commit the TypeScript types to make this fix, or is this also an issue with the Flow types?

Just published 1.7.4 which fixes the DomAttribute issue being described.

Unfortunately, Circle CI appears to be down so it says the build is failing, which it is not. Either way, the code should be working as expected

Can confirm I no longer have the issue on 1.7.4.

Thanks for replying so quickly @jamesmfriedman; great library! Happier to be using this with TS now 😄

Awesome. Keep using the issue if you find any other problems.

Just a headsup: a union with React.HTMLAttributes works, but doesn't make the type complete. Because in the underlying MDC you can use tag to change the HTML tag, you oughta match the tag value to the correct html attributes.

I'm just mentioning this so you know why it's not recognised correctly if you use a field on React.SpecificAttribute which is not present on the base attributes.

As far as I know there is currently no good way to solve this in either flow or TS, without making all the calls generic and have you put in the html attributes type which defaults to the base attributes.

@SleeplessByte You're absolutely right. I did think of this, but came to the same conclusion. I'm not really sure how this can be done, even with generics since tag can be manipulated at runtime. Thats why I defaulted to using any with the HTML props.

Definitely open to any help on this.

DrawerHeader seems to still miss the HTMLAttributes.
Edit: Also the Drawer prop "open" is required. Shouldn't this be true as default?

If you have the HTML Attributes type as the default generic type, you can
leave it up to the user. Just keep all you have right now, but replace the
exact type by a default type with Type Parameter.

```JSX
BaseComponent
````

I chose A for attributes here instead of T for type.

And yeah you can change tag at runtime, but that becomes the user's issue
hehe. They would use a or (type | other type) in their type and decide with
casting which one is being used.

On a sidenote, somewhere in the react definitions there is a giant map from
type to attributes. Theoretically you can capture tag not as a string but
as a enum value. You then map the enum value to a type. Or I think that
could work. Or maybe not. :P

On Mon, 25 Jun 2018, 23:29 James Friedman, notifications@github.com wrote:

@SleeplessByte https://github.com/SleeplessByte You're absolutely
right. I did think of this, but came to the same conclusion. I'm not really
sure how this can be done, even with generics since tag can be manipulated
at runtime. Thats why I defaulted to using any with the HTML props.

Definitely open to any help on this.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/jamesmfriedman/rmwc/issues/40#issuecomment-400101898,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB35WJbjj_ZO7D4egyiSLOYVq4VXTtiKks5uAVYrgaJpZM4QwOjC
.

@lal12 fixed and published with 1.7.5

It seems types of dependencies aren't published properly. Any typing packages for dependencies must be included in rmwc's dependencies (see https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html).

Currently I'm stuck on this error, even after installing @types/material__snackbar:

/Users/nick/Repos/stackselect/node_modules/rmwc/Snackbar/index.d.ts
(6,29): Could not find a declaration file for module '@material/snackbar/dist/mdc.snackbar'. '/Users/nick/Repos/stackselect/node_modules/@material/snackbar/dist/mdc.snackbar.js' implicitly has an 'any' type.
  Try `npm install @types/material__snackbar` if it exists or add a new declaration (.d.ts) file containing `declare module 'material__snackbar';`

I’ll look into that. It doesn’t show up on projects that allow implicit any from imports. This is one of the configuration things I thought might come up since tsconfig has quite a few settings dealing with imports.

I use "strict": true, which includes noImplicitAny.

@nickmccurdy could you do me a favor and post your tsconfig? That will definitely help troubleshooting.

I'm using the tsconfig from react-scripts-ts, with some modifications to make it more strict.

My customized config: https://github.com/nickmccurdy/stackselect/blob/master/tsconfig.json
Original: https://github.com/wmonk/create-react-app-typescript/blob/master/packages/react-scripts/template/tsconfig.json

@nickmccurdy I've gone ahead and added the most recent version of @types/material-components-web to rmwc. It's published in 1.7.6

Even before installing the additional types, I was unable to create any compile errors using your exact config, everything just worked as expected. I to am using react-scripts-ts with create-react-app. Even though I can't repro your error, I'm inclined to believe you since I still don't know much about TS :). Let me know if this fixes the problems you were hitting.

Thanks for the quick response. Unfortunately, I still get the same error after updating to 1.7.6.

Steps to reproduce

npm create react-app my-app --scripts-version=react-scripts-ts --use-npm
cd my-app
npm install rmwc @types/prop-types
# add "import 'rmwc';" to src/index.tsx
npm start

Hey @jamesmfriedman.

So I started to dig in and have run into a couple of roadblocks. (It's important to note that we are working from a custom Webpack config and not create-react-app.)

First, compilation is successful but results in these warnings

Second, including the Button component results in this error:

withFoundation.js?6da3:231 Uncaught TypeError: Cannot read property 'prototype' of undefined
    at withFoundation (withFoundation.js?6da3:231)
    at eval (index.js?0bf1:103)
    at Object../node_modules/rmwc/Ripple/index.js (main.js:12651)
    at __webpack_require__ (main.js:64)
    at eval (withRipple.js?6c9d:14)
    at Object../node_modules/rmwc/Base/withRipple.js (main.js:12591)
    at __webpack_require__ (main.js:64)
    at eval (index.js?3c4e:16)
    at Object../node_modules/rmwc/Base/index.js (main.js:12531)
    at __webpack_require__ (main.js:64)
    at eval (index.js?23a1:20)
    at Object../node_modules/rmwc/Button/index.js (main.js:12615)
    at __webpack_require__ (main.js:64)
    at eval (AppLayout.tsx?8f50:1)
    at Module../src/layouts/AppLayout/AppLayout.tsx (main.js:13633)
    at __webpack_require__ (main.js:64)

And lastly, attempts to use the Provider results in this TS error:

JSX element type 'RMWCProvider' is not a constructor function for JSX elements.

It's quite possible that the issues are unique to our implementation - our version of @types/react ("15.6.2") is not consistent with our version of react (16.4.1) and I have a hunch that this is fallout from that.

But just wanted to make some progress in getting this into our app.

Keep up the good work man!

Hi ya!

The first error is a postcss issue with root element properties, from the
base MDC library. I think they have an issue in their repo.

When your types don't match your base react, you'll implement calls
wrongly. Don't know if that is the issue here, but I'd start with
normalising that :)

On Fri, 6 Jul 2018, 19:20 Shawn Rice, notifications@github.com wrote:

Hey @jamesmfriedman https://github.com/jamesmfriedman.

So I started to dig in and have run into a couple of roadblocks. (It's
important to note that we are working from a custom Webpack config and not
create-react-app.)

First, compilation is successful but results in these warnings
https://gist.github.com/shawnatxapp/e27ddd0436270286cf1db88227325b7f

Second, including the Button component results in this error:

withFoundation.js?6da3:231 Uncaught TypeError: Cannot read property 'prototype' of undefined
at withFoundation (withFoundation.js?6da3:231)
at eval (index.js?0bf1:103)
at Object../node_modules/rmwc/Ripple/index.js (main.js:12651)
at __webpack_require__ (main.js:64)
at eval (withRipple.js?6c9d:14)
at Object../node_modules/rmwc/Base/withRipple.js (main.js:12591)
at __webpack_require__ (main.js:64)
at eval (index.js?3c4e:16)
at Object../node_modules/rmwc/Base/index.js (main.js:12531)
at __webpack_require__ (main.js:64)
at eval (index.js?23a1:20)
at Object../node_modules/rmwc/Button/index.js (main.js:12615)
at __webpack_require__ (main.js:64)
at eval (AppLayout.tsx?8f50:1)
at Module../src/layouts/AppLayout/AppLayout.tsx (main.js:13633)
at __webpack_require__ (main.js:64)

And lastly, attempts to use the Provider results in this TS error:

JSX element type 'RMWCProvider' is not a constructor function for JSX elements.

It's quite possible that the issues are unique to our implementation - our
version of @types/react ("15.6.2") is not consistent with our version
of react (16.4.1) and I have a hunch that this is fallout from that.

But just wanted to make some progress in getting this into our app.

Keep up the good work man!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/jamesmfriedman/rmwc/issues/40#issuecomment-403096408,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB35WOJpsC76ziloDqsgSrmYNF4FeeRvks5uD5xSgaJpZM4QwOjC
.

Just wanted to follow up on my comment above

The runtime error was happening because of a weakness in our webpack config. The resolve.extenstions array looked like this:

extensions: [".css", ".sass", ".scss", ".ts", ".tsx", ".js"],

Which was pulling in Ripple.css instead of the Ripple.js file and wreaking havoc. Reordering the array to look for the ts/js files first seemed to do the trick:

extensions: [".ts", ".tsx", ".js", ".css", ".sass", ".scss", ],

So lesson learned- order matters.

The Provider issue mentioned above still stands, but we can live without that so I'm not concerned with fixing it at this time.

A good time to follow up, 1.8 has some good TS updates thanks to @mutsys. Please report and failures (or successes) for validation.

Also, could definitely use some guidances on how to create tests to validate TS is working. Right now, the test is just that it compiles successfully, I'm not sure if anything more than that needs to be done.

It looks like the Snackbar type declaration in DefinitelyTyped is for @material/snackbar/index.js, but rmwc/Snackbar/index.d.ts is importing it as @material/snackbar/dist/mdc.snackbar', so it has an implicit any type. It still errors if I install @types/material__snackbar because of the mismatch.

@nickmccurdy

This is a great question, how do we handle conflicts like that? This example brings up one of the reasons I wanted to bake TS definitions directly into this codebase since some of the packages on definitely typed can be wrong.

  • Is it possible for you to declare that module yourself as any so at least it's not implicit?
  • MDC is on version 0.37.1 but the stuff of definitely typed is only on version 0.35.0. I don't think this will be a huge problem since MDC isn't exposed directly to the consumer through RMWC, but I'm expecting at some point I'll hit compilation errors if that stuffs not up to date.
  • Should we just declare all of those modules manually as any and avoid using definitely typed for material all together?

I wouldn't recommend copying dependency type declarations, there should be a single source of truth for type definitions for each dependency. @types packages can be added to the package's dependencies to ensure they are installed.

@nickmccurdy maybe I did a bad job explaining

Currently the types are being pulled from definitely typed. My issue is, definitely typed is behind MDC and not often correct.

Also, what you brought up about the Snackbar should technically impact everything in RMWC, but it sounds like that problem is isolated to Snackbar?

If types for a dependency package are incorrect, we should fix it in the
appropriate types package. DefinitelyTyped is pretty quick to review
contributions. However I'm not sure if the dependency path issue is with
rmwc or DefinitelyTyped, we should probably figure it out first.

On Fri, Jul 20, 2018, 5:48 PM James Friedman notifications@github.com
wrote:

@nickmccurdy https://github.com/nickmccurdy maybe I did a bad job
explaining

Currently the types are being pulled from definitely typed. My issue is,
definitely typed is behind MDC and not often correct.

Also, what you brought up about the Snackbar should technically impact
everything in RMWC, but it sounds like that problem is isolated to Snackbar?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/jamesmfriedman/rmwc/issues/40#issuecomment-406735428,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4l9Me166a_X67ZqmcX6GUdz02N3F0_ks5uIlArgaJpZM4QwOjC
.

I can confirm that Snackbar support is broken.

import { Snackbar } from "rmwc/Snackbar";
**/node_modules/rmwc/Snackbar/index.d.ts
(6,29): Could not find a declaration file for module '@material/snackbar/dist/mdc.snackbar'. '**/node_modules/@material/snackbar/dist/mdc.snackbar.js' implicitly has an 'any' type.
  If the 'material__snackbar' package actually exposes this module, consider sending a pull request to amend 'https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/material__snackbar`

@nickmccurdy Any existing solution?

@mattferrin while RMWC ships with its own types, it relies on @types/material-components-web for the MDC typings. Unfortunately, the definitely typed typings haven't been getting any community love. My suggestion:

  • Contribute to @types/material-components-web and bring it up to speed. RMWC will pick up the latest minor release automatically when you install.
  • Declare the Snackbar module in your own project and cast it to any. Before you tell me that this is a terrible idea, your project will never be directly exposed to the library since it's obfuscated by RMWC. Declaring the module is just a formality to get your compiler to stop complaining about it.

Two troubles:

Could not find Snackbar declaration:

code as:

import { Snackbar } from "rmwc/Snackbar"

report from TypeScript compiler:

ERROR in [at-loader] ./node_modules/rmwc/Snackbar/index.d.ts:6:29 
TS7016: Could not find a declaration file for module '@material/snackbar/dist/mdc.snackbar'. '/home/king/Project/builder-browser/node_modules/@material/snackbar/dist/mdc.snackbar.js' implicitly has an 'any' type.
Try `npm install @types/material__snackbar` if it exists or add a new declaration (.d.ts) file containing `declare module 'material__snackbar';`

resolved by:

rename '@material/snackbar/dist/mdc.snackbar' to 'material__snackbar' -- node_modules/rmwc/Snackbar/index.d.ts line 6

module @types/material-components-web defines that terms.

Bad constructors definition

code as:

import { TextFiled } from "rmwc/TextFiled"

report from TypeScript compiler:

ERROR in [at-loader] ./node_modules/rmwc/TextField/index.d.ts:205:50 
TS2510: Base constructors must all have the same return type.

ERROR in [at-loader] ./node_modules/rmwc/TextField/index.d.ts:255:44 
TS2510: Base constructors must all have the same return type.

resolved by:

  1. delete new <P>(props: Readonly<any>) -- node_modules/rmwc/TextField/index.d.ts line 163
  2. delete new <P>(props: Readonly<any>) -- node_modules/rmwc/TextField/index.d.ts line 213

duplicated definition

@tulayang thanks for digging into this some. If could could help provide a bit more guidance

  • The snackbar issue, these seems to be an issues with the definitely typed repo. I'd rather fix it there. Hoping the community will jump in and keep that up to date as I cannot take that on.
  • On the TextField issue, I finally managed to repro and have corrected it (as well as the same issue elsewhere) throughout. Interestingly enough, I use RMWC in a TS project of my own and never ran into this issue, I was just able to get my IDE to complain about it after running a build in RMWC.

For anyone following this issue, had a great tip off that the TS defs contained a lot of extra junk. I've been able to make some refinements to the build process to remove all of the garbage.

Would love some validation when someone gets a chance. 2.0.2.

The 5.x.x release has been completely re-written top to bottom in Typescript for React 16.3 and higher. For anyone that is using the current generated Typescript implementation, or anyone that wanted to try the library but found the old type system broken, now is the time to try out the 5.x.x-alpha branch and leave your feedback!

Very happy to close this issue and to fully support Typescript! Enjoy the V5 release.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jordoh picture jordoh  Â·  3Comments

Anwardo picture Anwardo  Â·  6Comments

web2033 picture web2033  Â·  4Comments

filbertteo picture filbertteo  Â·  3Comments

sangdth picture sangdth  Â·  5Comments