Add TypeScript types

[lilyvelour]

Adds TypeScript definitions file to the package.

See #200 for discussion

[ryanbrainard]

@lmontoute Awesome, thanks for adding this! I don't know anything about TypeScript, so I'm going to rely on other folks who do. Perhaps @rjhilgefort or @hburrows (hey, long time no see!) could lend a hand?

As an aside, we should probably also add Flow types at some point.

[rjhilgefort]

@lmontoute's TS expertise is beyond mine, but it looks good to me. Much more complete than what I had created for myself and covers what I covered plus a lot more. Thanks for submitting this @lmontoute!

Forgive the potentially stupid question. Is it idiomatic to submit something to https://github.com/DefinitelyTyped/DefinitelyTyped as well?

[lilyvelour]

@lmontoute's TS expertise is beyond mine, but it looks good to me. Much more complete than what I had created for myself and covers what I covered plus a lot more. Thanks for submitting this @lmontoute!

No problem!

Forgive the potentially stupid question. Is it idiomatic to submit something to https://github.com/DefinitelyTyped/DefinitelyTyped as well?

Either option works. Keeping the types in the same repository helps ensure (theoretically) that they're up to date, and is recommended especially if the package is written in TypeScript. It also makes package consumption much easier, as developers won't have to independently search for types.

Making a PR to DefinitelyTyped for a @types/[name] package is useful if the repository owners choose not to have the types included with the package.

Never both at once though, as it will lead to conflicts.

It's mainly a question of maintenance, especially if a package has an evolving API. Even though this package isn't written in TS, it might be worth keeping here if Flow types are being considered as well (that way, they can easily be kept in sync). Up to you folks of course :)

[ryanbrainard]

Keeping them here seems to make more sense to me. I'm assuming this would be easier on developers using these types so they don't have to depend on somehow https://github.com/DefinitelyTyped/DefinitelyTyped?

Does this need a release for people to pick up the changes? I'm guessing so, but wondering if we can get back with just a merge.

[hburrows]

I second bundling the type declarations here. I've noticed a trend since using TS that more and more type definitions are migrating from DefinitelyTyped to their package. I view DefinitelyTyped as a "transitional" place for declarations to exist before being adopted by the package proper. I'm swamped but will look closely in the next day or 2.

[hburrows]

@ryanbrainard This will need a release. After merging I'm happy to use it directly and give it some burn in. I have a couple pages that are going to use both react-refetch and react-redux and I'm curious if the HOCs will play nicely together.

[ryanbrainard]

@lmontoute thanks again for this! This has been released in 2.0.1. I skipped going through a beta phase since this doesn't impact existing code.

@hburrows want to give it a spin and see how it goes?

[rjhilgefort]

@ryanbrainard I'm working with react-refetch on a TS project right now, so I'll bump to 2.0.1 and let this thread know if I find any holes or issues in the type defs.

[ryanbrainard]

Awesome, thanks @rjhilgefort

[ipanasenko]

I've installed 2.0.1, but I don't see types bundled with it. Am I missing something?

[ryanbrainard]

@ipanasenko oh, you're right !🤦‍♂️
@lmontoute (or anyone else who knows TypeScript), is there something we need to change on the build/publish to make this end up in the released artifacts?

[lilyvelour]

Ah, I missed this! It seems index.d.ts just needs to be included under files in package.json. Or alternatively the type definitions can be moved, but a types field would need to be added to package.json (ex: "types": "./lib/main.d.ts")

References:
https://docs.npmjs.com/files/package.json#files
https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#including-declarations-in-your-npm-package

[hburrows]

Personally, I like using the types property. The intention is very clear and unambiguous.

[ryanbrainard]

I like types too. I'll open a PR for that. I'm a little concerned about how that'll play with Flow types, but we can cross that bridge if/when we need to.

[ryanbrainard]

So, I tried this out, and it seemed to not be an either/or thing. I had to add it to files for it to be included in the published artifact (tested with npm pack):

diff --git a/package.json b/package.json
index c81f763..6e7a92a 100644
--- a/package.json
+++ b/package.json
@@ -3,6 +3,7 @@
   "version": "2.0.1",
   "description": "A simple, declarative, and composable way to fetch data for React components.",
   "main": "./lib/index.js",
+  "types": "./index.d.ts",
   "scripts": {
     "build": "babel src --out-dir lib",
     "clean": "rimraf lib dist coverage",
@@ -17,7 +18,8 @@
     "url": "https://github.com/heroku/react-refetch.git"
   },
   "files": [
-    "lib"
+    "lib",
+    "index.d.ts"
   ],
   "keywords": [
     "react",

Is this correct?

Another thing I'm wondering about is if the types should go in src next to index.js. That seems better me, but not sure if that's prefered.

[lilyvelour]

Good to know! I wasn't 100% sure if both or just one of these were needed.

As for file location, I don't believe there are any issues in putting the file in a location that fits the project.

For example, we provide a library for our API with a lot more types. Instead of a single index.d.ts in the root, we break the types down into individual files + an index.d.ts in a types folder.

[ryanbrainard]

Proposed solution in #220