Nightly bug(?) with infering function generic while being part of intersection

[toodols]

First of all, the title exists for the sake of a title. I don't really know exactly what is wrong with this bug. I came across this while working on something with redux tookit.

Error: Parameter 'state' implicitly has an 'any' type.
Expected: state = WritableDraft<{username: string, isLoggedIn: true, ...}>

I am using the nightly extension and the bug has not occured because of an update to redux toolkit or a change in my code, but has instead been because of typescript versions changing.
The code itself works in typescript 4.7.0 but not in typescript nightly/4.8.0

In order to create a minimum reproducible example, I have tried to simplify it as much as possible, (which was harder than I imagined). The problem is rather weird, there are multiple parts that cause this bug and removing any will fix it. However, the issue remains, version 4.7.0 works while the same on nightly errors.

At this point, I still do not know exactly what the problem is. In fact, it may even be intentional. However, this is something that breaks redux toolkit so I think the matter is something worth taking a look at.

What search terms have I used? None. I don't know how to put this bug into words.

[RyanCavanaugh]

Taking possible red herrings out of the repro:

// @strict: true

declare function createSlice<T>(
    reducers: { [K: string]: (state: string) => void } & { [K in keyof T]: object }
): void;

createSlice(
	{
		f(a) {}
	}
)

a is string in 4.7 and implicit any in 4.8 and I don't see a good reason for that to have changed.

[andrewbranch]

@typescript-bot bisect good v4.7.4 bad main

[typescript-bot]

The change between v4.7.4 and main occurred at 9236e39.

[andrewbranch]

The PR that caused the behavior change was #48668.

Taking a closer look at @RyanCavanaugh’s simplified repro, it’s not so clear to me what the expected behavior really should be. Both before and after #48668, T is inferred from the mapped type to be { f: unknown }, so the instantiated apparent type of reducers is

{ [key: string]: (state: string) => void } & { f: object }

If you wanted to say that the type of the property f of that type was object & ((state: string) => void), I think that would be reasonable, but that doesn’t seem to be how we treat intersections of concrete properties and index signatures. We consistently say the type of property f of this type is just object... except in this repro pre-#48668. It looks to me like the 4.7 behavior is oddly inconsistent. Even quick info has some puzzling results:

If the contextual type of f is object, how can the contextual type of its parameter not be an implicit any?

To be clear, I don’t think the implicit any is desirable per se, but I think it’s consistent with the inference being made and the general behavior of getting types of concrete properties from an object intersected with an index signature. To get rid of the implicit any, I think we’d either need to

1. Infer {} for T
2. Make ({ [k: string]: T } & { p: U })['p'] → T & U

I don’t know what heuristic we could use to decide to do (1)—it seems super sketchy. (2) makes theoretical sense to me but would be a massive break and probably have tons of undesirable follow-on effects (I’m sure there’s an issue open about this).

TLDR, it’s not great that the behavior changed to introduce an error, but I can’t quite figure out why it ever worked before. Is there a logic to the past behavior I’m missing?

[andrewbranch]

A possible rebuttal of my analysis that I thought about but forgot to address: you might say that getting a property of a contextual type is fuzzier and more permissive than getting a property of that same type via something like a property access expression. For example, if I have a union type string | { x: (state: string) => void }:

type T = string | { x: (state: string) => void };
declare let a: T;
a.x; // Error because `x` is not on all constituents of the union

const y: T = {
  // `state: string` - we simply ignored the unuseful constituents when
  // doing the "same" operation on a contextual type
  x(state) {}
}

However, if the only relevant distinction was "getting a property of a contextual type is not the same as getting a property of a type generally," we would expect this to work:

let x: { [key: string]: (state: string) => void } & { f: object } = {
  f: (a) => {} // Error: `a` is implicitly `any`
};

and it does not. There is something different happening not just because there is a contextual type, but because of... well, something else around that mapped type mapping over keyof T.

[RyanCavanaugh]

Andrew and I kicked this around a while and think the right move on this particular repro is Won't Fix. Reasons for this:

• The thing that broke it changes other things in demonstrably good ways
• In the given repro, there's just no reason to write that index signature, since it only presumably subsumes the behavior of the existing typing since all functions are objects (and in cases where this is not the case, the error is correct). The one place it doesn't is symbol keys, and if you write T in (keyof CaseReducers) & symbol]: {}, then the code works as hoped-for
• We don't really know how to fix it

So anyway if you have a repro that falls under both the "intersection is necessary" and "contextual parameter inference would be sound" umbrellas, we can take a look at that in a new issue. Thanks!

[lukeapage]

Redux Toolkit is the recommended way of doing redux and is highly used. The current documentation (https://redux-toolkit.js.org/usage/usage-with-typescript) is that when creating a slice, the state parameter does not need to be typed, as it will infer from the initial state.

You can see this error also on your New Errors bug: #50028 - the redux counter example now fails.

I'm afraid I'm not enough of an expert to fully get what you are saying - are you suggesting that the redux toolkit types should be fixed to work with 4.8 by not having { f: object } ? Since you have narrowed down the example I'm finding it hard to work out where in redux toolkit this is? somewhere here? https://github.com/reduxjs/redux-toolkit/blob/master/packages/toolkit/src/createSlice.ts#L227

or are you saying that this infer will no longer work and everyone using redux needs to copy-paste a state type argument on to every reducer case?

I'm going to create a issue with redux-toolkit and see if they have any better luck in figuring this out.

[lukeapage]

I tried to cleanup ValidateSliceCaseReducers to this:

export type ValidateSliceCaseReducers<S, ACR extends SliceCaseReducers<S>> = ACR & {
    [T in keyof ACR]: ACR[T] extends {
        reducer(s: S, action?: infer A): any;
    }
        ? A extends PayloadAction<any>
            ? CaseReducerWithPrepare<S, A>
            : never
        : CaseReducer<S, PayloadAction<any>>;

but it didn't help...

[andrewbranch]

Hmm, I actually think Ryan’s simplified repro may have simplified too far: in the simplification, the first intersection constituent was an object with an index signature, whereas in the real case, it is a type parameter constrained to such an object. My analysis was based on the specific behavior we see in getting properties from an intersection like that. But in the actual redux-toolkit types, I can remove the constraint to the index signature and the behavior looks the same. So I think there’s something deeper going on here.

I will try to investigate a bit more. As a last resort, it certainly seems like we would be ok to revert #48668 if we can’t fix this before 4.8 is released.

[andrewbranch]

Simplified a little less:

type SliceCaseReducers<State> = {
    [K: string]: (state: State) => State | void;
};

type ValidateSliceCaseReducers<S, ACR extends SliceCaseReducers<S>> = ACR & {
    [T in (keyof ACR)]: ACR[T] extends {
        reducer(s: S, action?: infer A): any;
    } ? {
        prepare(...a: never[]): Omit<A, 'type'>;
    } : {};
};

declare function createSlice<State, CaseReducers extends SliceCaseReducers<State>>(
  options: {
    initialState: State | (() => State);
    reducers: ValidateSliceCaseReducers<State, CaseReducers>;
  }
): void;

export const clientSlice = createSlice({
	initialState: {
		username: "",
		isLoggedIn: false,
		userId: "",
		avatar: "",
	},
	reducers: {
		onClientUserChanged(state) {
			// ...
		},
	}
});

[andrewbranch]

But in the actual redux-toolkit types, I can remove the constraint to the index signature and the behavior looks the same.

I was wrong about this.

Minimal repro with the type parameter constrained to a record:

type Validate<T> = T & { [K in keyof T]: object }
declare function f<S, T extends Record<string, (state: S) => any>>(s: S, x: Validate<T>): void;

f(0, {
  foo: s => s + 1,
})

It looks like here, when we are trying to contextually type s from T & { [K in keyof T]: object } where T extends Record<string, (state: S) => any>, we are pulling exclusively from the mapped type template. (I can change object to be a function taking a parameter of some conflicting type, like string, and that contextual type “wins,” creating an assignability error at the call site.) So this is, after all, the same behavior I described earlier that is pervasive in intersections with index signatures. I don’t think it’s good, and it looks particularly bad here as a constraint, but it still is more consistent with how we handle index signatures elsewhere.

It looks like @phryneas has solved this on redux-toolkit, but we should keep an eye out to see if other things are impacted by #48668 too.

[phryneas]

Thanks for the further research!

It was also a bit of an error on our side, since the intention was never to & object the properties of T that were functions - but only those that were objects in the first place. But the sudden change was definitely very irritating.

I also tried to just create a mapped object that would pick only those properties that we wanted to restrict - but in that case I got any again, so it really seems to concentrate on the mapped object, mostly ignoring the original properties.

There is also more (probably unrelated to this) breaking in 4.8, in the "generic" use cases, but that's a lot less pressing than this one was - I'll see if I can investigate that further anytime soon.

[typescript-bot]

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of this repro running against the nightly TypeScript.

---

Comment by @RyanCavanaugh

❌ Failed: -

• Parameter 'a' implicitly has an 'any' type.

Historical Information

Version	Reproduction Outputs
4.3.2, 4.4.2, 4.5.2, 4.6.2, 4.7.2	👍 Compiled

[typescript-bot]

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of this repro running against the nightly TypeScript.

---

Comment by @andrewbranch

❌ Failed: -

• Parameter 's' implicitly has an 'any' type.

Historical Information

Version	Reproduction Outputs
4.3.2, 4.4.2, 4.5.2, 4.6.2, 4.7.2	👍 Compiled

[typescript-bot]

The change between v4.7.4 and main occurred at 9236e39.

[Meligy]

You can see a workaround for the redux case in user-land (where redux toolkit is the user of TypeScript) in this PR:
reduxjs/redux-toolkit#2547

[markerikson]

Hi, I'm another Redux Toolkit maintainer. Wanted to follow up on this.

Lenz has put together a potential workaround on our side, but it requires what is frankly a pretty ugly hack (a separate package that abuses typesVersions to figure out what TS version we're dealing with, and changing what our createSlice type includes based on that).

Any thoughts on whether this issue is something the TS team intends to address before 4.8 is released?

[RyanCavanaugh]

I've been looking at createSlice today to try to figure out if there's something that can be done in userland to avoid the problem.

The originating use case at #48812 is a lot more compelling due to its simplicity, but I don't like breaking Peter to fix Paul.

[markerikson]

@RyanCavanaugh thank you! Lenz is on vacation atm, but if you've got any questions or anything, please ping me . Appreciate you at least looking at this!

[RyanCavanaugh]

Is there a test suite or equivalent where I can try different definitions of createSlice to make sure I'm preserving the typing of the use cases?

[markerikson]

Yeah. We have both unit tests, and a set of "type tests".

• https://github.com/reduxjs/redux-toolkit/blob/master/packages/toolkit/src/tests/createSlice.test.ts
• https://github.com/reduxjs/redux-toolkit/blob/master/packages/toolkit/src/tests/createSlice.typetest.ts

to run them:

cd packages/toolkit
# Run just the createSlice unit tests
yarn test createSlice.test 
# Run _all_ the typetests
yarn type-tests

[Andarist]

Hmm, what's the current status of this particular issue? Is it considered to be a valid use case? I've found myself being somewhat confused by index signatures in more complex scenarios and I don't know how to properly reason about this.

Note that both latest reduced repros from @andrewbranch can be fixed by using T[K] within the mapped type's template (either by ending up with it in the conditional type there or by intersecting with it): nightly TS playground

[phryneas]

@Andarist unfortunately, the T[K] would break createSlice in older TS versions, so it's not a fix that would work for us on it's own. So we now end up adding another conditional to decide between {} and T[K] depending on the TS version.

[markerikson]

@RyanCavanaugh : just to check, what was the final resolution? Just a reversion of the commit that caused this breakage for us?

[Andarist]

Since this has been fixed by reverting the other fix - could we reopen #48812 ?

[RyanCavanaugh]

@markerikson correct - reverted in both 4.8 final and ongoing in main. Hopefully a fix can be found that satisfies both use cases.

@Andarist done 👍

[markerikson]

Thank you for taking a look at this!