search

rnc-archive / .github

This repository contains the general guidelines for the RNCommunity org
https://github.com/react-native-community
200 stars 31 forks source link

Standardise type definitions #14

Open xzilja opened 5 years ago

xzilja commented 5 years ago

Right now we have several repos that include both flow and ts definitions, or just single one or none.

It would be awesome to come up with guidelines for adding these definitions into the repositories (i.e what folders should they go in, what file names should be used etc...)

Some definitions will probably need to be created from scratch, some can be ported from @types/react-native package (since there are now packages that are untyped due to extraction from main rn repo).

mikehardy commented 5 years ago

There was some discussion on this just recently on Discord in the meta channel. The most supported position there was (quoting @ericlewis) - 

Best option: use typescript by default with strict settings so your library is 
automatically typed as you work (and for everyone who wants to use it too)

Second best option: maintain your own types, if this feels annoying, think about what 
you could get from 1st option and consider doing that. Otherwise, your types probably 
don’t change that often and if they do then option 3 isn’t great either.

Option 3: use DefinitelyTyped, probably the worst option out of the bunch and anyone 
using typescript will tell you it’s the worst option. It can often be wrong, it takes 
time, it’s a separate dependency, it doesn’t get upgraded in lockstep with your 
library (unless the user thinks to)

There were three thumb-up and no dissent, just as information. I think encouraging Typescript use and at least including the types (with type checks during CI etc) is a reasonable stance, personally

rborn commented 5 years ago

Adding the initial PR and proposal that triggered all this.

Both for not to forget after we reach a decision and to check the opinions expressed there.

PR: https://github.com/react-native-community/react-native-maps/pull/2877 Proposal: https://github.com/react-native-community/react-native-maps/issues/2847

matt-oakes commented 5 years ago

The quote from @ericlewis is exactly correct. Added types to DefinitelyTyped should only be done as a last resort if a package maintainer refuses to add them to the package itself. It's a much better developer experience for them to be included either by writing the library in TS or with a bundled definition file.

ericlewis commented 5 years ago

Just to clarify on option one: it’s “automatically” typed in the sense that because you need to use types if you are using typescript (hence strict mode) then you’ll be able to output quality and accurate types at compile time. Furthermore, your library will also benefit from type safety. If your library is just export default NativeModules.Foo then there isn’t much difference between step 1 and 2 really... however, by typing your interface directly (like we did when making all the flow specs for NativeModules) instead of maintaining a definition file manually, you open yourself to potential free future optimizations such as automatically generating TurboModule interfaces.