expand oparator have wrong typing

[dilame]

Describe the bug

Expand operator have wrong typings of project function. From the description it applies the projection function to every source value as well as every output value, so the project function should receive both – input and output types. But actually the types are implemented like this

export function expand<T, O extends ObservableInput<unknown>>(
  project: (value: T, index: number) => O,
  concurrent = Infinity,
  scheduler?: SchedulerLike
): OperatorFunction<T, ObservedValueOf<O>>

That means i can return another type from the project function and will not receive a compilation error.

Expected behavior

The type should be like this

export function expand<T, O extends ObservableInput<unknown>>(
  project: (value: T | ObservedValueOf<O>, index: number) => O,
  concurrent = Infinity,
  scheduler?: SchedulerLike
): OperatorFunction<T, ObservedValueOf<O>>

Reproduction code

No response

Reproduction URL

No response

Version

7.6.0

Environment

No response

Additional context

No response

[LBBO]

Shouldn't it be more than that? Since the original events are just passed through as well, shouldn't the return type be OperatorFunction<T, T | ObservedValueOf<O>>? Here's a short showcase that demonstrates that both types are currently incorrect.

Also, I'd love to implement this!

[dilame]

Oh, yes, seems you are right!

[LBBO]

Alright, I did some experimenting and it seems like that type would loose type inference. At least I can't figure out a way to have O be inferred here, which leads to both it and the final observable's result being unknown. I tried to break the issue down and came up with this TS playground which also showcases the solutions I am about to suggest. Please let me know if anyone has an idea how to type this correctly while still preserving full type inference! Also, I'm not 100% sure my simplification is fully applicable to the actual problem, so please also let me know if you find an issue with that.

Assuming that type inference cannot be preserved, the next question would be how users should provide the uninferable type. Sadly, TypeScript doesn't support partial type argument inference at the moment so I only see the following options:

1. expand<InputType, OutputType>(project) - provide both type arguments explicitly. This is unnecissarily verbose as the InputType can be inferred.
2. expand((x: InputType | OutputType): ObservableInput<OutputType> => ...) - make sure the project function is explicitly typed. This could also be done as something like expand((x: number | string) => typeof x === 'string' ? of(x.length) : EMPTY) where the return type can be inferred from the explicitly typed parameter
3. Refactor expand to make it curried as a workaround for achieving partial type argument inference. The result might look something like expand<number>()(x => typeof x === 'string' : of(x.length) : EMPTY), where the input type could be inferred as string and we just manually provide the output type number. The obvious downside to this is that the way expand is called would even change in the resulting JS code, not just the TS code

No matter how this issue is fixed, it would be a breaking change either way since existing code might work with the current typing and break with the new, correct type (even if no workaround was necessary in order to preserve inference). However, I think this still needs to be fixed since the current type is simply wrong.

Before I put any more time into this issue, I'd like to ask for some feedback from maintainers. At the very least, I need to know which approach to take, but I can also imagine that I overlooked something or that this is a wontfix for some reason.

[dilame]

@LBBO seems like i resolved it 😄