Evaluate current Redux TS port status to determine remaining work for a 5.0 release

[markerikson]

Pasting from #4127 :

What are the breaking changes in 5.x? Are the types different enough that it needs a new major? What's holding it back from getting released?

That's sorta the issue - we're not sure what the breaking changes are, it's likely the types would benefit from more changes, and there's not enough reason to push it live and cause churn in the ecosystem atm :)

The conversion work was actually done in... late 2019, I think? And it's been sitting there in master ever since.

Basically, the community contributed the work after we asked for it, and then the whole effort just stalled.

There's also some tie-in to redux-thunk's types being stuck in a holding pattern as well.

Basically, what's live in 4.x right now works well enough, and especially with RTK now being the default, that there just isn't enough pressing reason to try to push through a 5.0 release any time soon.

I'd like to see 5.0 released, but it needs some meaningful attention to figure out what other types changes might be necessary, and given everything else going on it's just at the bottom of the priority list for us right now.

If someone else with serious TS knowledge wants to help us out and try to push the current TS port forward by evaluating it and seeing where things stand and how it can be improved, I'd love it.

[ryota-murakami]

It's difficult question what feature deserve it for bump up major version.
TypeScirpt conversion has been completed with almost 95% accuracy or better.

Personally stability is strongest point in the state management libraries world.
Therefore fitting much better No New Feature strategy for Redux rather than ExperimentalUnstable hype the core.

Lodash last release was 2016 but still feels like active.
So that's enough continue to follow future TypeScript/ECMAScript new elements, standard? 🤔

[markerikson]

We could consider releasing this as a 4.2.

Part of the issue is the whole debate over breaking changes in TS types. Ideally, there would be no breaking changes in our types and we could just do a release. But, it's also very likely that some change made in the conversion process will break people's code. On the flip side, TS itself doesn't follow semver and new TS releases themselves can break existing code at times.

The questions atm are:

• Do the current changes in this conversion right now have any issues or breaking problems?
• Are there any other changes we should make to improve the types, even if they are actually going to be breaking changes?

[timdorr]

We most definitely have breaking changes for TS users. We have to make the next release a major version bump.

[Methuselah96]

One of the new changes that's in master but not in 4.x is #3507 which introduced a return type to replaceReducer for the purposes of typing. As proposed in #3772 I think we should seriously consider reverting that change before it gets released since it's unclear whether it would actually make it possible to type the pattern that is trying to be typed and it doesn't seem to fit with the overall architecture of Redux (and it seems that @timdorr has concerns as well).

[markerikson]

FWIW I just published https://github.com/reduxjs/redux/releases/tag/v4.2.0-alpha.0 from master so we can get feedback on real-world types compat.

[Methuselah96]

@markerikson Yeah, I saw that which is why I brought it up since I really don't think we should release 4.2.0 before merging #3772 so that we don't add a return type that would require a breaking change to remove in the future.

[markerikson]

Sure. To be clear, I don't intend to actually shove a 4.2.0 release out the door any time soon :)

I'm just noting that since we were able to put out Reselect 4.1 with updated types as a minor, and this rewrite has been sitting around for two years now, I wanted to get a published alpha live to give people something to test out and push this issue forward.

[Methuselah96]

@markerikson Absolutely. Thanks for getting the ball rolling. The addition of a return type to replaceReducer is in fact a breaking change for people writing enhancers (e.g., redux-devtools in my case) since it would require the enhancer to return the new store when overriding replaceReducer.

[markerikson]

Suggestion from https://twitter.com/sangster/status/1454563925480116225 :

Switching from a default of AnyAction to UnknownAction (ie, replace “any” with “unknown”) would be a breaking change but would likely help a lot of folks find bugs they didn’t know they had.

[Methuselah96]

This might be controversial, but I think it would be more helpful to make it so that the type has to be a string. When converting redux-devtools to TypeScript I've been using Action<unknown> for the action which results in a lot of casting action.type to string. These casts are covering up "bugs" since my understanding is that officially action.type can be any type (even though the docs say it should be a string) and I suspect that something would break if someone tried to use a different type.

Any thoughts about restricting action.type to string? It seems to be more practically useful in my opinion to restrict the type to string since it's a lot easier to write tooling with the assumption that the action types are strings and it is how Redux is practically used in real-world usage at the moment.

[markerikson]

I don't think that's "controversial" at all :) tbh I'm actually surprised the types don't enforce that already. (I'd also be mostly surprised to see any real-world code out there at this point that is trying to use Symbols or something else, but given the weird code I've seen, I guess it's probably that someone is doing that).

Frankly we oughta just have createStore check for typeof action.type === 'string' too at this point.

[timdorr]

I'm actually against that. There is no design enforcement of what an action should actually contain, other than a type property of some sort.

I think what we can do is rather than default to any for the type property, we default to string. That would align with the expected defaults, but still give an escape hatch for those that want to use Symbols or other non-string things.

[markerikson]

Yeah, I'm saying we should start having that "design enforcement".

We've always said they should be at least serializable:

As with state, serializable actions enable several of Redux's defining features, such as time travel debugging, and recording and replaying actions. Using something like a Symbol for the type value or using instanceof checks for actions themselves would break that. Strings are serializable and easily self-descriptive, and so are a better choice. Note that it is okay to use Symbols, Promises, or other non-serializable values in an action if the action is intended for use by middleware. Actions only need to be serializable by the time they actually reach the store and are passed to the reducers.

In practice they're always strings. I can't see a good reason for them to not be strings.

[nycdotnet]

We have used enums in the past which basically boils down to number. This would intuitively be more memory efficient than a string but I have no measurements to back that up. I have no interest to block this idea that they must be strings, and this was way before TS supported union types with string literals. Figured I'd let you know, though, of one non string idea out there. Thanks for maintaining Redux!

[timdorr]

In practice they're always strings. I can't see a good reason for them to not be strings.

That is true. Hmmm... 🤔

I guess it's a thematic thing for this release: Do we want to start being more strict about things (unknown instead of any, type: string, etc)? If it's in the name of less error-prone user code, I'm all for that.

[markerikson]

Yeah, like I quoted above, we've always said they should be serializable, and ought to be strings.

Someone over on Twitter said they'd once tried to use Symbols to make them truly unique, but tbh slice namespacing gives you like 95% uniqueness anyway, so I don't see that as a reason to continue to allow them. As mentioned above, TS enums can have number values, but again why would you do that? We want them to be readable in the DevTools.

So, given that we've always effectively said they should be strings, we might as well enforce it.

And stuff like that definitely qualifies as a major :)

As far as the original topic for this thread: I've been working with @JoshuaKGoldberg from CodeCademy on some Redux+TS updates to their codebase, and he just confirmed that redux@5.0.0-alpha.0 dropped in with no issues whatsoever. That's after we fixed some TS issues that did come up with 4.0.5 -> 4.1.1.

Just a single data point, but it suggests that the TS types in master are pretty consistent with 4.x.

[Methuselah96]

Instead of creating an UnknownAction and using it as the default we should just get rid of AnyAction and use Action as the default. Creating an UnknownAction with an index signature of [extraProps: string]: unknown still has the problem of not giving an error for accessing a property that maybe shouldn't exist and using it in a way where the type doesn't matter For example, if (action.something != null) would not produce an error with UnknownAction when I think it would be better if it did produce an error by default if action.something isn't defined.

[markerikson]

Update since someone asked on Discord:

The notional plan atm is:

• RTK 1.9 is feature-complete, we "just" need to wrap up a few additional bug fixes and cleanup bits (docs, figuring out how to publish codemods, etc). No hard ETA, but ideally we'd like to get that out in the next couple weeks.
• After that, it's time to start actively thinking about RTK 2.0. This will primarily be about dropping backwards compat stuff (IE11, etc): Future planning: RTK 2.0? redux-toolkit#958 . Definitely no ETA on that because I don't know scope yet, but it's the next major thing to work on.
• As part of that, I would like to finally put out Redux 5.0 as part of the RTK 2.0 release. I still don't know if Redux 5.0 actually breaks any TS types, so this issue is still a valid open question. But, if we're ever going to get that out, doing those releases together makes sense

[markerikson]

Vaguely related: for Redux 5.0 we should convert listeners into a Set. We can safely assume ES6 support by now :)