Port to TypeScript

[nickmccurdy]

Do you want to request a feature or report a bug?

No

What is the current behavior?

Redux publishes a manually created index.d.ts file. This also affects reselect, redux-devtools, and redux-thunk.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar.

Explanation of why this isn't recommended: https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

To summarize, type declarations should either be generated from TypeScript source code or shared in DefinitelyTyped, which allows contributors to edit the types without having to edit the original package. This can also help alleviate maintenance and versioning issues regarding Redux users that write their code in TypeScript. Many issues about TypeScript compatibility could be moved to the dedicated DefinitelyTyped community if the types were separated, and the types could have breaking changes released without interfering with non-TypeScript users.

What is the expected behavior?

Redux either removes its type declaration (should be a breaking change) and moves them to DefinitelyTyped (recommended) or is ported to TypeScript (could be difficult and requires additional maintenance, but makes the types more accurate).

Which versions of Redux, and which browser and OS are affected by this issue? Did this work in previous versions of Redux?

3.4.0+ (all versions that publish index.d.ts)

[jamestharpe]

I'd like to see redux ported to TypeScript because TypeScript is the only way to ensure types are always in sync.

Types that are always in sync prevent issues from being reported, improve the overall developer experience, and increase the overall potential for adoption of the library.

The general benefits of types are that they improve the quality and productivity of the developer experience, even for non TypeScript developers. Types help catch bugs, enable better tooling, and enhance understanding.

Types provide these benefits most effectively when they perfectly match the codebase. Leaving types to a separate community all but ensures a lag between library changes and the related type definition changes, making it an overall less effective approach to providing types and therefore lessening the overall value of the library.

Regarding effort, because TypeScript is a superset of JavaScript the code base can be converted gradually. Gradual conversion means ongoing efforts are only minimally disrupted. Converting to TypeScript can also be done as a nonbreaking change, meaning it is also less effort for developers to upgrade to.

Finally, TypeScript it's self has a vibrant community. Conversion to TypeScript may garner additional contributions to help with the effort.

[markerikson]

Relevant Twitter discussion thread:

https://twitter.com/acemarke/status/1160243663370371073

[markerikson]

@jamestharpe : realistically, Redux is not going to be ported to TS any time soon.

Tim and I don't have enough TS experience to do the porting or maintenance ourselves, and even if someone else does the work, that basically makes the code impossible for us to maintain. That's already happened over in Redux Starter Kit - it was ported by several users, and now I am effectively unable to make meaningful changes to the code myself.

[janhesters]

@markerikson Redux might be small enough to learn TypeScript 😊

That being said, I prefer types bundled with the package instead of DefinitelyTyped. The reason is simple. It's a sync issue. If the types are bundled with the package, they are usually updated with breaking changes and additions as opposed to after. I've helped moving the React Navigation types into the package for exactly that reason.

[ricokahler]

Coming from a typescript fanboy, I don’t think redux should be ported to typescript especially if the maintainers don’t have a strong TS background.

I personally think the typing should be removed from this library and moved to definitely typed. I think the community will do a great job maintaining them.

Just look at react itself. Definitely typed manages those and they’re great. Given the popularity of this library, I don’t think there will be much lag time between versions even.

[timdorr]

I've got a pretty handle on our types at this point. There are some things I would like to add (more inferred types, for instance), but those are nice-to-haves, not things that make the types unusable or inaccurate. I do get the concerns around independent versioning, but I don't think there's any particular need to update them right now, so that's not really a practical concern.

I've actually already tried this with redux-thunk and it's a complete rats nest. The linting alone was a giant hassle; other dependent packages hadn't been updated in a while and were out of compliance with the latest config. And there are a lot of things that rely on redux that are going to trip those same set of issues. It's a giant mess to unravel.

[nickmccurdy]

Overall I think moving to DefinitelyTyped is the best option. Yes, TS generates more accurate types, but it’s too much work for Redux to migrate without a rewrite. We can improve the current state of typings by maintaining them separately. Bundling like we’re doing now is not recommended by Microsoft.

[markerikson]

@janhesters : I have learned TS.

I just don't know TS well enough to be able to do stuff like this:

redux/index.d.ts

Lines 375 to 384 in 63dda81

	export type StoreEnhancer<Ext = {}, StateExt = {}> = (
	next: StoreEnhancerStoreCreator
	) => StoreEnhancerStoreCreator<Ext, StateExt>
	export type StoreEnhancerStoreCreator<Ext = {}, StateExt = {}> = <
	S = any,
	A extends Action = AnyAction
	>(
	reducer: Reducer<S, A>,
	preloadedState?: DeepPartial<S>
	) => Store<S & StateExt, A> & Ext

or this, from Redux Starter Kit:

type CaseReducerActions<CR extends SliceCaseReducers<any, any>> = {
  [T in keyof CR]: CR[T] extends (state: any) => any
    ? PayloadActionCreator<void>
    : (CR[T] extends (state: any, action: PayloadAction<infer P>) => any
        ? PayloadActionCreator<P>
        : CR[T] extends { prepare: PrepareAction<infer P> }
        ? PayloadActionCreator<P, string, CR[T]['prepare']>
        : PayloadActionCreator<void>)
}

type NoInfer<T> = [T][T extends any ? 0 : never];
type SliceCaseReducersCheck<S, ACR> = {
    [P in keyof ACR] : ACR[P] extends {
        reducer(s:S, action?: { payload: infer O }): any 
    } ? {
        prepare(...a:never[]): { payload: O }
    } : {

    }
}

[ee0pdt]

For libraries that are not written in TS I’ve noticed a worrying lag in types being added to definitely typed vs maintained in the repo. I’ve also seen more bugs and errors in the typings which lead to issues getting raised in the library even though these only relate to the types.

Based on the latest JS surveys and extrapolating out I would expect that this year around 55% of JS devs will have used TS and be happy to use it again. At the point that the majority of JS devs are actually TS devs, every JS library needs to consider TS as a majority use-case. This means eventual death to any lib that does not have good typings available.

But, I do appreciate that types for non-trivial libraries are a massive undertaking so I do see why the maintainers would want to discuss this.

[oriSomething]

If the maintainers aren't TS users, I think maintaining types as part of release is problematic because:

1. It's pain releasing patch versions only because in types mistakes, I saw it a lot in immer which sometimes there where 3 releases in a row only for types fixes
2. As someone who contribute to DT, I think it's less painful than contribute to projects themselves especially when the maintainer isn't a heavy TS user
3. For popular projects, mostly DT definitions are better than project's original when it's not TS. For example, React written in Flow, but as someone who uses both Flow and TS, mostly TS definitions that created by community are superior

[jamestharpe]

Given the maintainer’s lack of interest in porting to TypeScript, I think DT is the way to go.

• Lag should be roughly the same
• Redux package size will be slightly smaller
• Fewer issues/concerns for Redux maintainers will help focus on new features

[nickmccurdy]

It seems like moving to DefinitelyTyped is preferred by most users and maintainers.

@markerikson @timdorr How would you like to proceed? I’m thinking we could create a branch/PR that removes Redux types, then PR the Redux package types to DefinitelyTyped (may involve some extra code style and test work to meet their review criteria). Once that’s published, it should be safe to merge the removal of the declaration file as a major release (since it’s a breaking change for TypeScript users).

[markerikson]

FWIW, Daniel Rosenwasser from the TS team says they should be moved to DT:

https://twitter.com/drosenwasser/status/1160628533166628864

I personally would be in favor of moving them to DT, but I know @timdorr has expressed interest in keeping them here. So, I'll leave it up to him to decide.

[markerikson]

All right, so I'll throw this out there just to bring it up:

@timdorr , do we ever foresee the rest of the Redux libs getting rewritten in TS?

I would assume no, but just want to get your thoughts on that for the record.

[nickmccurdy]

I think the benefits of maintaining code and types together would outweigh the cost of migrating to TypeScript for smaller packages like redux-thunk and reselect (which has extremely complex types that are harder to test because they're separate from the source code). It still might backfire if the maintainers don't have TypeScript experience, but they're simple enough that they shouldn't go very out of sync.

[timdorr]

My concern with switching anything to TS is approachability. Even after transpilation and bundling, our code is still quite readable. And given the size of the library and its importance in the ecosystem, that's something I do not ever want to compromise on.

In addition, our types are also pretty well documented and quite functional at this point. Again, we really only have improvements to make to them, not fixes. So other than the decorum of moving them elsewhere, I don't see that changing how much they're developed or often they're released.

For redux at least, I'm pretty much fully against this. It makes a lot more sense for things like redux-thunk (which I had huge problems when I last tried it...), but not for this small, central library.

[ricokahler]

In addition, our types are also pretty well documented and quite functional at this point. Again, we really only have improvements to make to them, not fixes. So other than the decorum of moving them elsewhere, I don't see that changing how much they're developed or often they're released.

For clarification on my above comment, if there is no real problem, then I wouldn't fix it. My vote is to do what @timdorr wants to do.

[timdorr]

[nickmccurdy]

@timdorr Understood. Are you comfortable with moving the existing type declarations to DefinitelyTyped? Work has already started in #3501.

[timdorr]

Just making a copy of them? Sure, but I'm not sure if that would create problems for the other type packages that depend on them.

[nickmccurdy]

No, I mean deleting them from Redux as a breaking change and major release. TypeScript only recommends publishing types when the package is written in TypeScript.

[timdorr]

No, I said I'm fully against that. I want to keep them here.

[nickmccurdy]

Can we discuss why? This is not recommended by the TypeScript documentation and I think it’s been causing versioning and maintenance issues with Redux. A Microsoft employee on the core TypeScript team has specifically suggested we move to DefinitelyTyped. Removing types is the simplest way to solve the problem without porting the code to TypeScript (which I don’t think is a good idea in this situation).

[gengjiawen]

Keep typing here will save us version problem and easy to change.

[timdorr]

I've already stated my reasons above.

Honestly, DT is just a maze of tooling full of landmines and gotchas. It's a half-gig of git data with 6000+ types and nearly 3000 open issues. It's extremely hard to get into, and the tooling that's in place doesn't make it particularly easy to get started. When I tried to move in the redux-thunk types (one project where I do support the move), I couldn't figure it out. All the docs were unhelpful and the tooling gave no reasoning for why unrelated types were failing.

Again, one thing I really like about Redux and is very important to its community, is its approachability as a code base. DT is the opposite of this, IMO. Don't get me wrong, I think it's great to have a central clearinghouse for type definitions for projects unaware of TS. But the nature of the Redux community runs counter to that.

I think the perception of TS being a maintenance issue might be due to the past few years where I didn't know any TS and relied heavily on the community. I'm at the level where I may not be the best TS coder, but I think I've gotten pretty good. Certainly good enough where I can offer more meaningful review of type PRs.

For all intents and purposes, Redux is "done" right now. We don't have plans for another major (including the types), so the types can be the driver on moving the version numbers forward. That's totally fine in my mind. It doesn't create a maintenance problem because the actual code we distribute stays the same and we can easily check that it's not changing from release to release (I always run npm pack before publishing a release).

One thing that might be an interesting experiment would be a parallel implementation of the code in TypeScript. Basically, move the types out of the definition files to the code itself, side-by-side with the plain JS implementation. That's definitely a more complex setup, but it may make the type def generation easier and less like the fancy examples that Mark posted above. Again, this is made feasible by the code being "done" and unlikely to receive major changes in the future.

Either that or we do actually rewrite into TS and have some pre-commit hooks to transpile down to plain JS and store that in the repo.

Let me do a Twitter poll for science.

[timdorr]

https://twitter.com/timdorr/status/1161278569038974977

[nickmccurdy]

TypeScript is designed to keep separate types in DefinitelyTyped, it's still a better option even if it's more difficult to use at first. DefinitelyTyped will get easier to contribute to over time as it matures, and it already has some very useful tooling that makes this kind of type work easier. Keeping types here involves extra maintenance churn even if we don't care about version numbers, and prevents us from using the latest tools and configuration without setting them up manually.

Also, I don't think we should have a second implementation in TypeScript, unless it has breaking changes to the API. If we can manage to rewrite Redux with full API compatibility (and our tests should make that easy to determine), we can just replace the JS implementation with the TS one, and it will still be fully usable by JS users. TypeScript packages are actually published as plain JS, with the TS declaration files automatically generated, so they work well with both languages.

@timdorr Before I forget, how do you feel about handling this for reselect, redux-devtools, and redux-thunk? Would you have the same decision as here, or would it be useful for me to open similar issues for discussion specific to those repositories?

[MichaelDeBoey]

I think we should avoid using any and try to use unknown instead (where possible).

This is the main value proposition of the unknown type: TypeScript won't let us perform arbitrary operations on values of type unknown. Instead, we have to perform some sort of type checking first to narrow the type of the value we're working with.

https://mariusschulz.com/blog/the-unknown-type-in-typescript

Happy to make a PR if wanted. 🙂

[cellog]

@MichaelDeBoey by all means, yes.

There are a few cases where any is used in order to allow for the huge range of possibilities (state values, for instance), but some of these can be tightened up. For instance, I don't think it would be too restrictive to require state to be object | string | number.

One example where any is probably the right choice: when writing a store enhancer that just passes preloadedState unmodified to the underlying store, any seems appropriate, as it doesn't interfere with the typing of createStore or the return value typing of the store enhancer. What do you think?

I'm not sure unknown is what we want for cases like the definition of an action. This would force users to type their actions strictly, and in many cases where people are moving from javascript to typescript, this will require huge PRs right off the bat with a bunch of boilerplate. That would have stopped the last company I worked at from even considering the move. Again, curious about your thoughts on this

[timdorr]

We tell people to only put "serializable" data into the store. So, things that JSON.stringify can work with. But, despite our warnings, people still put all manner of silly things in there.

As a practicality, I don't think we can restrict to objects and scalars.

I also agree that while unknown is probably more proper, it's a giant headache for users who already complain enough about Redux's boilerplate.

🎉 New in Redux 5.0! More boilerplate!

[markerikson]

Yep. Please don't constrain the state type.

[cellog]

note that DeepPartial is no longer used. #3485 removed its usage, but did not remove it from index.d.ts. I think we should delete it since it serves no useful purpose, and note that as a breaking change in case anyone was importing it. It's a 1 liner, they can just copy/paste to use it:

export type DeepPartial<T> = {
  [K in keyof T]?: T[K] extends object ? DeepPartial<T[K]> : T[K]
}

Any objections?

[timdorr]

Yeah, I was waiting for the next major to drop it, since we export it and others may be using it. It can go for 5.0.

[jednano]

Hey @timdorr and @markerikson, please let me know if I can help at all with this TypeScript conversion. I'm super comfortable with TypeScript and I don't want you guys to struggle. I also realize you're using this as a sort of learning opportunity, so best of luck!

I know you already know the TS basics, so I want to recommend that the most important features of TS you need to learn are generics and conditional types. Master these concepts and you will be a TypeScript god.

Fortunately, the TS documentation is super good.

[timdorr]

And we are now converted to TS! 💃

[markerikson]

This effort seems to have stalled a bit.

Per discussion with Tim, it sounds like the main conversion work is done, and now it's a question of what improvements can be made to the TS types (including potentially breaking changes).

What potential types improvements are we thinking of?

If the compose changes from #3568 are really an improvement, I'm fine with somebody resurrecting those as a new PR (as long as the discussion stays focused on just the types aspects).

I know there's also some discussion about ActionTypes.INIT in #3580 .

Anything else?

[cellog]

I don't think #3568 is a minefield worth navigating unless someone runs into a bug with the existing types.

I agree fully that #3580 needs to be fixed.

I would encourage doing a release after that and then fixing with point releases, it's the only way to know how the changes affect people :)

[timdorr]

Does anyone know what test/typescript/ is for now? It doesn't appear to be used in any tests or linter runs. It's also chock full of errors. Is it left over from the conversion and didn't get cleaned up? Should we try and fix the errors?

[cellog]

yeah.. you removed it :)

4a8eca8

I strongly suggest reverting this commit. typings-tester allows verifying that types actually fail when they should, something that is impossible otherwise.

[cellog]

I feel I was a bit hasty in tossing off a quick reply. Let me elaborate.

I used the tests in tests/typescript while porting typescript over. They caught many breakages that I would have otherwise missed when making "minor" changes to the types. I was not familiar with typings-tester prior to working on this, and I became a big fan because of this. I think that this is the only way to make a fix that this bug requires while ensuring nothing else breaks.

There are, of course, probably other ways to do this, but since the infrastructure is already here, I recommend using it.

[cellog]

one last comment - the only problem in typescript.spec.ts was the comment, which read:

import { checkDirectory } from 'typings-tester'

describe('TypeScript definitions', function() {
  it('should compile against index.d.ts', () => {
    checkDirectory(__dirname + '/typescript')
  })
})

and should be something like:

import { checkDirectory } from 'typings-tester'

describe('TypeScript definitions', function() {
  it('should verify types fail when appropriate as well as working with normal usage', () => {
    checkDirectory(__dirname + '/typescript')
  })
})

[nickmccurdy]

Use dtslint instead, it's officially supported by Microsoft.

[cellog]

nice! following the link at dtslint which says "if you only need ExpectType and ExpectError" we get https://github.com/SamVerschueren/tsd

What do you think about using that @nickmccurdy ?

[nickmccurdy]

Not sure, I haven’t heard of it, but sounds worth trying

[jednano]

Use neither.

If you are writing the library in TypeScript, don't use dtslint. Use --declaration to have type definitions generated for you.

https://github.com/microsoft/dtslint#setup

The basic point is that you don't have to "test the types" if the types were generated, as you can trust they are accurate. This simplifies libraries that are written in TypeScript.

Please, don't reintroduce these types of tests.

[nickmccurdy]

Ah, I missed that. Have we made sure those test cases are tested in our new TypeScript tests?

[markerikson]

@jedmao : I'd disagree with that. We're using some additional types tests over in Redux Starter Kit to make sure the types actually behave the way we want, and they've found several important problems. Example:

reduxjs/redux-toolkit#209

[cellog]

Again, the tests check for things that are not otherwise possible to test for. Simply assuming they work without testing that they actually fail on invalid cases is risky for a library

[nickmccurdy]

@cellog Are you suggesting that we should assert both type errors and runtime errors using dynamic tests?

There are some scenarios where types may be incorrect even if the runtime behavior is correct. I think @markerikson has a good point that we should still keep some sort of type tests for invalid cases.

[jednano]

That's a good example, but I wouldn't apply it wholesale across the entire library. Just where it's relevant.

Looks like tsd is the better choice, as it's limited in scope and doesn't use the antiquated TSLint.