Using createSelector with EntityAdapter's getSelectors causes '$CombinedState' error

[akramhussein]

Problem

When using the EntityAdapter's exported selectors with reselects createSelector a Typescript error is thrown regarding the $CombinedState symbol not being exported.

Example:

import { createSelector } from '@reduxjs/toolkit'

import { sessionsAdapter } from '../reducers/sessions'
import { RootState } from '../types'

export const { selectAll: selectAllSessions } = sessionsAdapter.getSelectors((state: RootState) => state.sessions)
export const currentSessionSelector = createSelector(selectAllSessions, (sessions) => sessions[0])

Results in the following error

Exported variable 'currentSessionSelector' has or is using name '$CombinedState' from external module "../node_modules/redux/index" but cannot be named.ts(4023)

Package Versions

Current:

• typescript 4.5.5
• redux 4.1.2
• @reduxjs/toolkit 1.7.2
• react-redux 7.2.6

This code was previously working with the following package versions:

Previous:

• typescript 4.4.3
• redux 4.1.2
• @reduxjs/toolkit 1.6.1
• react-redux 7.2.6

I'm not sure at which point it broke as I upgraded to the latest and I'm now working backwards.

[akramhussein]

I believe this may be a reselect issue as it happens with another selector that is not using an EntityAdapter - please let me know if you want me to migrate it to there.

[markerikson]

Hmm. I'm not sure if it is or not. $CombinedState is a Redux core type.

[phryneas]

That happens because you are building declarations.

If you don't need them, you can turn that off in your package.json. If you do need them, adding redux as a dependency will usually shut down the error.

[akramhussein]

@markerikson @phryneas thanks for the quick reply.

@phryneas do you mean tsconfig.json? If so, I tried that, but unfortunately it's a monorepo whereby the redux code is in a sub-package that is being referenced. I tried setting declaration: false, however then composite does not work and if I set both to false the monorepo project won't compile. From what I read, I'll need declaration: true.

redux is already a dependency but I also tried add it to the other project referencing this one and also the root package.json, but also didn't help. Perhaps I'm doing something wrong here - will keep digging.

[akramhussein]

I tested pinning reselect to 4.0.0 and it fixed this.

[adnandothussain]

Any updates on it? I tried to downgrade the peer dependency reselect from latest to 4.0.0 and the issue resolved on my end

[phryneas]

This is a problem with three packages interacting with each other in a specific TypeScript setup - without a minimal reproduction I don't think we can do anything here (and even then it might be tricky)

[markerikson]

Yeah. What we really need is a complete project that actually reproduces the problem and shows this error happening.

Without that, there really isn't anything we can do.

[akramhussein]

Will try to create a reproducible example project. In mean time, I've identified that the breaking change occurred in 4.1.2 of reselect. Versions prior to this did not exhibit the issue.

Other versions:

• typescript 4.6.2
• redux 4.1.2
• @reduxjs/toolkit 1.6.1
• react-redux 7.2.6
• redux-persist 6.0.0

[markerikson]

We definitely iterated on the Reselect TS typings between 4.1.0 and 4.1.5. 4.1.5 should be "correct" behavior. Test against that.

[akramhussein]

Just tried - still shows the errors in 4.1.5. There wasn't a massive amount of code changes in https://github.com/reduxjs/reselect/compare/v4.1.1...v4.1.2 so I might try and check it out and see which part triggered the break so I can narrow it down.

[mpeyper]

I have also just run into this when reselect was updated from 4.0.0 to 4.1.5.

All selectors created with createSelector from @reduxjs/toolkit that are exported from a library are failing to build with tsc when declaration is true. Obviously we don't want to omit the declarations for a library.

[markerikson]

We really need a repro at this point :(

I'd also like to know if this happens with Reselect by itself as well.

(tbh I don't have much time to look at this myself right now. if anyone would like to investigate this, that would really be helpful. and if there's any chance of me actually looking at it, I need a full reproduction that I can clone and immediately see the error happening!)

[mpeyper]

Here you go. Just run:

npm i
npm run build

While setting this up, I'm 99% sure it's the use of ReturnType<typeof rootReducer> when overriding DefaultRootState (here) that is causing the issue (for me at least). I inherited the actual project so I'm not sure if this is a normal way of handling the root state or if there is a better, more idiomatic way to do it.

[markerikson]

Oh, yeah, you should never actually use DefaultRootState in practice.

That type was added to the community types years ago, and tbh I don't even know why it's there. It was probably added before unknown was part of TS.

This is part of why we specifically show actually inferring the real RootState from the Redux store's type, or at least from the root reducer.

What happens if you drop that part of it, and switch over to actually following the TS setup approach shown in our docs?

[markerikson]

Hmm. Okay, this does seem to be RTK-related somehow.

I tried switching the example over to use RootState, and the error persists:

import { createSelector } from "@reduxjs/toolkit";
// import { DefaultRootState } from "react-redux";

import type { RootState } from './store';

type AlternateRootState = {
  key1: string,
  key2: string
}

export const selector = createSelector(
  (state: RootState) => state.key1,
  (state: RootState) => state.key1,
  (key1, key2) => key1 + key2
);

but if I switch to AlternateRootState, it goes away.

selector was inferred as:

const selector: ((state: {
    readonly [$CombinedState]?: undefined;
    key1: string;
    key2: string;
}) => string) & OutputSelectorFields<(args_0: string, args_1: string) => string & {
    clearCache: () => void;
}> & {
    clearCache: () => void;
}

The actual Redux core TS types have this definition:

/**
 * Internal "virtual" symbol used to make the `CombinedState` type unique.
 */
declare const $CombinedState: unique symbol

/**
 * State base type for reducers created with `combineReducers()`.
 *
 * This type allows the `createStore()` method to infer which levels of the
 * preloaded state can be partial.
 *
 * Because Typescript is really duck-typed, a type needs to have some
 * identifying property to differentiate it from other types with matching
 * prototypes for type checking purposes. That's why this type has the
 * `$CombinedState` symbol property. Without the property, this type would
 * match any object. The symbol doesn't really exist because it's an internal
 * (i.e. not exported), and internally we never check its value. Since it's a
 * symbol property, it's not expected to be unumerable, and the value is
 * typed as always undefined, so its never expected to have a meaningful
 * value anyway. It just makes this type distinquishable from plain `{}`.
 */
interface EmptyObject {
  readonly [$CombinedState]?: undefined
}
export type CombinedState<S> = EmptyObject & S

I'll be honest and say I don't really understand what's going on here.

[markerikson]

Mmm... actually, if this is related to preloadedState... didn't we use to declare that it was DeepPartial, and then removed that because it actually isn't (ie, you can only provide top-level fields, and have to provide the full nested data for any of those)?

If so, does this $CombinedState thing even make sense any more?

but I also have no idea why the latest Reselect types are causing problems with this and declarations either.

[mpeyper]

The root of the problem seems to be combineReducers as the issue goes away if I construct the reducer myself. The latest reselect types work well when given hand rolled types or other inferred types.

That said, the upgrade of reselect did introduce a bunch of implicit any errors where before it seemed to infer the types of selectors from previous ones in the chain, but I actually think typing each one properly is somehow more correct.

[markerikson]

Yeah, the Reselect 4.1 types require that you be very explicit and exact about the input selector types, so it can correctly infer the types of the final selector arguments.

Sounds like this ultimately comes down to a Redux core types issue in combination with Reselect.

We'll have to dig back into this, but it's possible that this type may not even be needed any more.

@Methuselah96 , I know you've done some poking around the core types. Any knowledge of $CombinedState's background and if it's still needed?

[phryneas]

From what I understand, CombinedState is a phantom type marker that means "this comes from a combineReducer" to be able to use that information in preloadedState. So, essentially to distinguish between

createStore(reducerFunction<State>) // this can either have preloadedState of undefined or State, but not Partial<State> 
createStore(combineReducers({ a, b }) // `undefined`, `{a}`, `{b}` or `{a,b}`, but not a partial `a` or `b`
createStore(combineReducers({ a: combineReducers({ c, d }), b}) // `undefined`, `{a}`, `{b}` or `{a,b}`, and also partial `a`, but not partial `b`

[phryneas]

Now that I think about it, that should probably be just stripped from store.getState's result and that would solve a lot of problems.

[mpeyper]

I'm back from a few days holiday and diving into this again.

I'm a bit unclear why combineReducers gets special treatment compared to other reducer functions when it comes to preloadedState. Don't all reducer functions, including combineReducers, follow the same rules when it comes to initialising state?

Why should create migrating from:

const reducer1 = (state: string = "value1", action: AnyAction) => { ... };
const reducer2 = (state: string = "value2", action: AnyAction) => { ... };
const reducer3 = (state: string = "value3", action: AnyAction) => { ... };

export const rootReducer = combineReducers({
  key1: reducer1,
  key2: combineReducers({
    key1: reducer2,
    key2: reducer3,
  }),
});

to:

const initialState = {
  key1: "value1",
  key2: {
    key1: "value2",
    key2: "value3",
  },
};

export const rootReducer = (state = initialState, action: AnyAction) => { ... };

change how configureStore is allowed to be used?

Obviously this comes from an extremely naive perspective of not understanding how or why we got here, so just tell me it must be this way and I'll drop it.

[markerikson]

That's the thing. I'm not actually convinced this is necessary.

Going back in the Redux repo, the $CombinedState type was added in https://github.com/reduxjs/redux/pull/3485 , to fix https://github.com/reduxjs/redux/issues/2808 . This specifically mentions a DeepPartial type that was used with createStore.

The thing is, though: a Redux store's initial state cannot be "deep partial". It's only partial at the top level.

Tbh I'm very confused on whether $CombinedState is needed at all at this point.

[mpeyper]

For anyone reading this before a real solution is presented, I've taken the CleanState type from here:

import type { CombinedState } from '@reduxjs/toolkit';

type CleanState<T> = T extends CombinedState<infer S> ? { [K in keyof S]: CleanState<S[K]> } : T;

and dropped it in my project so I can wrap the RootState in it:

export type RootState = CleanState<ReturnType<typeof rootReducer>>;

With this, the project builds fine and the definitions for emitted types for the selectors and state are as expected.

[mpeyper]

Actually, the above workaround does remove issue with $CombinedState, but the new reselect types are still giving me all kinds of headaches for other types from external modules that cannot be named (to be honest, I don't really know what this error means or what it is about these types that is causing it).

I've discovered that I can work around the issue if I type the selector manually (bypassing the inferred types), but that kind of defeats the purpose of having the complicated types in reselect IMO.

I'm going to raise a new ticket in reselect repo for this as I feel like the new types there are the actual root of the issue rather than here, although I do think the whole $CombinedState should still be looked into.

[Methuselah96]

@markerikson The reason it's necessary is because the allowed preloaded state is different depending on whether you're using combineReducers or not.

A normal reducer's type is:

type Reducer = <S, A>(state: S | undefined, action: A) => S;

combineReducer actually creates a reducer with a slightly more permissive initial state:

type Reducer = <S, A>(state: Partial<S> | undefined, action: A) => S;

The introduction of $CombinedState was an attempt to fix this issue by saying "Hey, if I have the $CombinedState property then I'm allowed to be Partial, otherwise I can't be Partial."

The real solution would be to add a third generic to the reducer type which defines the initial state:

type Reducer = <S extends InitialState, A, InitialState>(state: InitialState, action: A) => S;

Then we could get rid of all of these $CombinedState hacks. However this third generic would spread to a lot of places and not look very pretty. Let me know if you want me to make a PR to show you what it would look like.

[Methuselah96]

https://github.com/reduxjs/redux/pull/4314 is a very rough draft of what it could look like.

[mpeyper]

combineReducer actually creates a reducer with a slightly more permissive initial state

The real solution would be to add a third generic to the reducer type which defines the initial state

This is sort of what I was hinting at with my comments above about this not being specifically a combineReducers feature, but rather any reducer function can be more permissive about what state it would accept vs what it will produce, so the types should either allow that for any reducer, or disallow it for combineReducers to be consistent across the board.

[markerikson]

Taking a step back for a bit:

is this maybe actually "just" an issue with Reselect itself? It sounds like we've established that this started happening with the Reselect 4.1.x type changes.

Does the same sort of error happen if you do something with a RootState type and some other usage that is combined with declarations: true?

[mpeyper]

No, I agree. The new types in reselect 4.1.x seem to be the problem here. Using RootState directly is not an issue elsewhere.