feat(ofType): narrow type of output based on action type

[thynson]

See #622 (comment) for detail

• I have read the Contributor Guide
• My commit messages are in conventional-changelog-standard format.

[jayphelps]

Thanks for the PR! Can you please add a detailed description on why this change is helpful and any downsides? The linked comment is helpful somewhat but not complete and also it's better to have everything here. Thanks again.

[thynson]

The reason behind the change is that a generic type param can only be inferred to literal type when it extends from a primitive type.

function foo<T extends string>(x: T): T {
  return x;
}

function bar<T>(x: T): T {
  return x;
} 

let x = foo('a'); // type of x is literal "a"
let y = bar('b'); // type of y is string

[MaximeBernard]

I see what you want to achieve and I think it's valuable for developer experience but I'm afraid the downside outweighs the upside.

I tried some Conditional Type

Type extends (Input['type'] extends string ? string : any),

but it doesn't work.

If you managed to make it work, we could go from here.

[MaximeBernard]

I don't think you can do that because

https://github.com/reduxjs/redux/blob/5f29b2667ff83f0189bc2aa304b214b4e19fa398/src/types/actions.ts#L18

export interface Action<T = any> {
  type: T
}

meaning it can be anything, not just a string (even though I agree the common usage is string)

If we do that, we won't be compatible with people not using string as their "type".

[thynson]

Hello, @MaximeBernard, thanks for your review.
I have updated it so that it should works for non-string action as well.

[MaximeBernard]

LGTM but I'm afraid I'm missing something. Any thoughts @evertbouw / @benlesh?

[evertbouw]

What happens if the type wasn't found? In other approaches it would silently narrow to unknown or never, something I'd like to avoid since it's not obviously wrong.

[thynson]

The template param Input extends from Action, so it should be rejected by the compiler. And on the situation that Input is inferred to non-literal type like Action<string>, the Output shall be the same with Input, which is the original behavior.

[evertbouw]

If you use a number that wasn't part of the union then it does error in the way I was afraid of. The ofType call looks fine but then in the map function you suddenly have a never. This can be confusing.

For strings it seems to work just fine. The vast majority of Redux users will use strings so I can accept this.

[MaximeBernard]

Coming back after a few days, I just realized:

function foo<T extends string>(x: T): T {
  return x;
}

function bar<T>(x: T): T {
  return x;
} 

const x = foo('a'); // type of x is literal "a"
const y = bar('b'); // type of y is literal "b"

Your previous assessment of string is due to you using let instead of const.

I've went a step further and tried

which seems to infer litterals just fine.

Am I missing something?

[thynson]

Yeah, Typescript seems to also infer something to literal type in a const context.

But in a real-world scenario, one has to add as const to make Typescript infer it as literals with the original ofType.

of<MyAction>().pipe(
  ofType('a' as const),
  map((x) => x.foo), // okay, x is `{type: 'a', foo: string}`
);
of<MyAction>().pipe(
  ofType('a'), // not inferred to literals without `as const` 
  map((x) => x.foo), // does not compiled, x is still `MyAction`
);

[evertbouw]

Closing old PRs. As always thank you for your contribution.

[thynson]

Any chance to get it reviewed and merged? @evertbouw
It just add extra function signature declaration to help the tsc to infer the type more accurately for DX improvement.