FormidableLabs / redux-little-router

A tiny router for Redux that lets the URL do the talking.
MIT License
1.04k stars 113 forks source link

Add TypeScript definitions for v15.0.0. #269

Closed parkerziegler closed 6 years ago

parkerziegler commented 6 years ago

This pull request adds TypeScript definitions for v15.0.0 of redux-little-router. It focuses on typing the high level aspects of the API, including actions, the Location and History interfaces, routers, and the Link and Fragment components. There is a demo repository created here to demonstrate using redux-little-router with TypeScript. The demo repository copies over the demo/client app, successfully passing the TS compiler. To run the demo locally, follow the instructions in the README.

screen shot 2018-02-11 at 9 49 42 am screen shot 2018-02-11 at 9 50 11 am

Type definitions were also created for server-side rendering, but have not been tested against a demo app. This will be important to do before shipping a new release.

Unfortunately, the types submitted to DefinitelyTyped don't fully support the API in its current state. Running the demo repository against these types results in the following Terminal output: screen shot 2018-02-11 at 10 12 43 am

I am concerned that folks using the API with the definitions on DefintelyTyped may run into issues where parts of our API that are supported in JS will not pass the TS compiler because of incorrect typings. Shipping our own TypeScript definitions with redux-little-router ensures we keep them current and consistent with the latest version of our API. I would love to have some folks review these types, critique them, and expand them if possible.

codecov[bot] commented 6 years ago

Codecov Report

Merging #269 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #269   +/-   ##
=======================================
  Coverage   98.61%   98.61%           
=======================================
  Files          28       28           
  Lines         362      362           
=======================================
  Hits          357      357           
  Misses          5        5

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 485cc4f...827d2b7. Read the comment docs.

parkerziegler commented 6 years ago

@tptee @ryan-roemer I think I'll need to add in defs for the immutable aspects of the API so the compiler won't yell about imports, I'll make the fix and push to this PR.

parkerziegler commented 6 years ago

@tptee @ryan-roemer Types have been added for the immutable aspects of the API. There is also a corresponding branch in the rlr-ts repo, immutable-demo, that verifies the types pass the compiler. webpack is correctly resolving the pointer to the ES build as well (redux-little-router/es/immutable/index.js). Let me know how I can help you out further!

screen shot 2018-02-12 at 9 53 24 pm

parkerziegler commented 6 years ago

@tptee We should be good to merge! I tightened up those definitions – they actually helped me to uncover an issue from the old typings that was over-restrictive on the data structures we could store in the routes object. Good eye! 🎉 💯