Skip to content

Improve type inference of pluck operator #2881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aboyton opened this issue Sep 28, 2017 · 5 comments
Closed

Improve type inference of pluck operator #2881

aboyton opened this issue Sep 28, 2017 · 5 comments

Comments

@aboyton
Copy link

aboyton commented Sep 28, 2017

RxJS version:
5.4.3

Code to reproduce:

const obs = Observable.of([{ a: { b: 1 } }]);
const a = obs.pluck('a');

Expected behavior:
a be of type Observable<{b: 1}>.

Actual behavior:
a is of type Observable<{}>

Additional information:
While I realise that I can provide the type of the return value to pluck, doing so doesn't give me any type safety. Instead, this could be inferred, at least for an arbitrary level of nesting. Anyone who goes beyond this level of nesting wouldn't be able to use type inference, very similarly to the current state of things.

type Pluck<T, K extends keyof T> = T[K];

export function pluck<
  T,
  P1 extends keyof T
>(
  this: Observable<T>,
  property1: P1,
): Observable<Pluck<T, P1>>;

export function pluck<
  T,
  P1 extends keyof T,
  P2 extends keyof T[P1]
>(
  this: Observable<T>,
  property1: P1,
  property2: P2,
): Observable<Pluck<Pluck<T, P1>, P2>>;

export function pluck<
  T,
  P1 extends keyof T,
  P2 extends keyof T[P1],
  P3 extends keyof T[P1][P2]
>(
  this: Observable<T>,
  property1: P1,
  property2: P2,
  property3: P3,
): Observable<Pluck<Pluck<Pluck<T, P1>, P2>, P3>>;

export function pluck<
  T,
  P1 extends keyof T,
  P2 extends keyof T[P1],
  P3 extends keyof T[P1][P2],
  P4 extends keyof T[P1][P2][P3]
>(
  this: Observable<T>,
  property1: P1,
  property2: P2,
  property3: P3,
  property4: P4,
): Observable<Pluck<Pluck<Pluck<Pluck<T, P1>, P2>, P3>, P4>>;
export function pluck<T>(
  this: Observable<T>,
  ...properties: string[]
): Observable<any> {
  return higherOrder(...properties)(this);
}
@kwonoj
Copy link
Member

kwonoj commented Sep 28, 2017

Surprisingly, our 5 version is based on TS compiler doesn't support keyof. it's dupe of #2295, while we will revisit this probably in 6 with bumped up TSC version (due to some changes around runtime inherited error etcs we consider bump up as breaking) but we can't do this in 5.

@kwonoj kwonoj closed this as completed Sep 28, 2017
@aboyton
Copy link
Author

aboyton commented Sep 29, 2017

Sorry, I'm a bit confused by the tickets. #2295 and #2179 are both closed (thus I didn't find them when searching). Is there an open ticket that's tagged with RxJS 6.0 so we make sure that this isn't forgotten about?

I noticed #2759 which has fixes to the type of an operator (mergeAll()) that's planned for the 6.0 release is still open.

@kwonoj
Copy link
Member

kwonoj commented Sep 29, 2017

Is there an open ticket that's tagged with RxJS 6.0 so we make sure that this isn't forgotten about

I'm planning to reopen one of original issue after we stabilize 6 branch and create separate label. For now it's closed as not actionable. There's mixed issues between some are opened and some are not and it's simply we haven't gone through all issues.

@aboyton
Copy link
Author

aboyton commented Mar 22, 2018

Is this ready to be reopened?

@lock
Copy link

lock bot commented Jun 5, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants