react-navigation / rfcs

RFCs for changes to React Navigation
Other
88 stars 16 forks source link

Migrate to TypeScript! -- Or at least internalize the typescript definitions #34

Closed fbartho closed 5 years ago

fbartho commented 6 years ago

Currently, TypeScript definitions for react-navigation are maintained by the community via DefinitelyTyped.

It'd be great if the TypeScript definitions were owned in house so they release in lockstep with API changes, and one way to do that would be to have the project itself be written in TypeScript.

I don't intend to give a whole sales pitch for TypeScript as that can be found elsewhere, but strong typing, with types for flow/typescript embedded in the repo would really smooth the learning curve, and help correct usage of the APIs.

There are people out there with tooling / willing to help with this project specifically, so it shouldn't be a giant burden on the core maintainers.

What would be people's thoughts about moving to TypeScript?

brentvatne commented 6 years ago

unless someone steps up and commits to being the person who will maintain the typescript type definitions this will not happen anytime in the near future. if you would like to do this, please open a pr to add typescript definition to react-navigation and also some way for them to be tested automatically on CI for each PR (see how for flow we run the definition against NavigationPlayground). I can add you as a collaborator to help you keep them up to date

fbartho commented 6 years ago

@brentvatne -- so just to be clear, the option you'd be suggesting would be side-written TypeScript types, that are manually kept in sync, rather than migrating the core codebase to self-generate the TypeScript interface?

If that is your suggestion, I'll have a think about my availability and answer next friday about such a commitment.

brentvatne commented 6 years ago

right. we recently removed flow from the codebase for some reasons described here: https://github.com/react-navigation/react-navigation/pull/3350 and I'm not super eager to add a comparable tool back right now. I'd prefer to just provide flow/typescript definitions and not actually use them to build the library for the moment -- in the future when the API/design of react-navigation stabilizes more and either flow stabilizes or typescript integration with react native is more smooth (eg: http://artsy.github.io/blog/2017/11/27/Babel-7-and-TypeScript/) we're open to reconsidering that! for now we lean more on unit tests

fbartho commented 6 years ago

Thanks for the context!

jhalborg commented 6 years ago

+1 for this. And just providing a d.ts file along with the repo, published along with SDK updates is fine for me, as a user.

I can't commit to being responsible though, as I have a baby on the way (as in due date this very day), so my availability in the next month(s) is more limited than usual. I can help out when things settle, though, at least with minor changes.

I hope @fbartho or someone else might be able to take on the task

brentvatne commented 6 years ago

congrats and good luck @jhalborg :)

jednano commented 6 years ago

I am wholeheartedly ready to contribute to this effort, whatever the cost.

I am concerned about manually typing the .d.ts files, because it doesn't have as much integrity as generated type definitions. I've already gone through this with the PostCSS library, where I converted the entire library to TypeScript just to generate perfect type defs that are now a bit stale because (back then) the author thought introducing TypeScript would deter community contributions somehow?

I think a good strategy for this would be to:

  1. Introduce TypeScript into the build system first.
  2. Configure it in such a way that TypeScript and JavaScript can co-exist.
  3. Incrementally convert existing JavaScript files into TypeScript, starting with the ones that have no dependencies and working our way up to the root.

I'll submit a PR with the build configuration and we'll see who likes it or not.

jednano commented 6 years ago

@brentvatne I just put up a PR to start this effort. See https://github.com/react-navigation/react-navigation/pull/4075

I can pull the d.ts file from DefinitelyTyped until the source files are generating more accurate type definitions.

I paved the way, so who wants to help me convert stuff to TypeScript!? @fbartho, @jhalborg, you in?

jhalborg commented 6 years ago

As much as I'd like to help out, it won't be possible within the foreseeable future on my part, unfortunately. I really do hope that some process can be built :)

brentvatne commented 5 years ago

tracking here: https://github.com/react-navigation/react-navigation-core/issues/2

if anyone wants to help out you can collaborate there