Skip to content
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

takeUntil unclear on how to coerce type #159

Open
keithamus opened this issue Jul 21, 2024 · 1 comment · May be fixed by #160
Open

takeUntil unclear on how to coerce type #159

keithamus opened this issue Jul 21, 2024 · 1 comment · May be fixed by #160

Comments

@keithamus
Copy link
Collaborator

keithamus commented Jul 21, 2024

takeUntil's IDL is as follows:

interface Observable {
  // takeUntil() can consume promises, iterables, async iterables, and other
  // observables.
  Observable takeUntil(any notifier);
}

However the first thing the spec for takeUntil(notifier) does with notifier, in step 2.3 is:

Subscribe to notifier given notifierObserver and options.

Subscribe assumes the given observer is an Observable. So I think there's a missing step to coerce an any to an Observable. Perhaps this is missing because from(any value) has yet to be specced? Additionally it looks as though Chrome's implementation just expects takeUntil() to recieve an Observable.


This is tracked in https://issues.chromium.org/issues/40282760 but I'm bringing it here also.

@keithamus keithamus linked a pull request Jul 21, 2024 that will close this issue
@domfarolino
Copy link
Collaborator

So I think there's a missing step to coerce an any to an Observable. Perhaps this is missing because from(any value) has yet to be specced

Exactly!

Additionally it looks as though Chrome's implementation just expects takeUntil() to recieve an Observable.

Right, we don't use from() + any because it is not finished being implemented for all of the planned coercions yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants