Typescript

[e111077]

I have converted the project to typescript. Adding typescript people to take a look. Also, I haven't chatted at all with the team on this (kinda just did this on my own) so I'd like to have a discussion on whether this is a optimal solution to adding typescript support to pwa-helpers.

Reviewers added: TS nerds and project owners

I tried to follow best-practices on redux using this guide: https://github.com/piotrwitek/react-redux-typescript-guide

but I'm still not sure if I have missed some concept here or there.

[justinfagnani]

I haven't chatted at all with the team on this

Seems like a critical thing to get agreement on. As much of a TypeScript fan as I am, all the maintainers should be fully on board before switching.

[e111077]

I agree, it was kinda an exercise for myself to 1. understand Redux and 2. learn how TS interacts with it. I'm totally okay with rejecting this PR since it complicates the demo quite a bit; there's no need or pressure to merge just because I made it. But it should be somewhat of a priority to support Typescript users, so instead of using all TS, an alternative can be to take the outputted .d.ts files and add those to the project for TS support

[merlinnot]

If you want to have some user voice on this... It would be great :) But if you decide to keep it plane js, we can always publish @types/pwa-helpers, so it's not a must-have :)

[e111077]

Well, we now have correct types that can be used!

[notwaldorf]

Hiiiiii ok so we did some offline chatting about this and it goes like this:

• we like types! we support adding types!
• at the moment this PR has no dependencies and no tool requirements, which makes it really nice and easy to use eeeeeeverywhere. We'd like to keep this as a feature
• as a result, we'd like to keep the source in js (so we would also not have to deal with building and praying the types away in the build-published-to-npm version
• based on this discussion (i know nothing about TS tools), i understand we have 2 options: 1) publish the .ts typings file to this repo, or 2) publishing them to @types.

We talked offline and your preference between these two seems to be for @types, so uhhh let's do that?

[merlinnot]

There is one difference: if we have codebase in TypeScript, our types are always up to date and we have (at least people who like TypeScript) better development experience.

We can add a prepublish hook to build it and we can easily get rid of the source if we put it into one directory (e.g. src) and add files to the package.json.

To be super safe, we can also add a precommit hook so no one commits stuff which is not compiling or does not pass tslint.

TypeScript is not only about consumers of this package, but also about people developing it.

I consider @types as the last resort.

@notwaldorf would you guys be more willing to merge this if I would improve this PR by adding a godlike config and a readme for people less experienced with TypeScript?

[notwaldorf]

@merlinnot this is the option 2 i listed, right? ("publish the .ts files to this repo). I asked the local TS experts, @e111077 and @rictic and they both suggested @types, so i defer to their decision.

I don't understand from your comment what "godlike config means", and whether this still means changing the source to TS -- this is something we are not planning on doing at this time.

[merlinnot]

If you are not planning to do that, than @types are indeed a better option. I was referring to your last sentence, "preference between these two seems to be for @types".

If we'd have a source in TS, than we wouldn't get out of sync, it happens a lot with @types.

For a config I mean something that is pleasant to work with and does not allow anyone to mess up :)

[notwaldorf]

Going to close this PR since my understanding is that the change isn't needed here (and it's needed in the @types repo instead). Pls reopen if I misunderstood our conclusion!

[dmarcuse]

My understanding is that DefinitelyTypes (@types) is usually considered a backup for when types aren't provided by the module itself. Considering how simple this module is, my opinion as a user is that it would be easier for everyone if declarations were provided with the module, which would require adding a .d.ts file and settings the types field in package.json. Publishing the declarations through @types would require submitting a PR with the same declarations to DefinitelyTyped every time they need to be updated, rather than simply updating a file in this repository, and would also add an extra step for TypeScript users (npm install --save-dev @types/pwa-helpers).

If you're interested, I'd be happy to open a PR adding the declarations.