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

Observable.from() & accept conversions #60

Merged
merged 5 commits into from
Sep 12, 2023
Merged

Conversation

domfarolino
Copy link
Collaborator

@domfarolino domfarolino commented Sep 7, 2023

This PR should close #28 and #44, by providing a static Observable.from() method, as well as changing takeUntil() — the only web platform method proposed here that takes "an Observable" — to accept the same kinds of objects that Observable.from() does.

I had some trouble initially writing this PR. For example: when used as an argument, WebIDL's Promise<T> type accepts native Promises as well as all thenables, I believe, which appears to be achieved by delegating to ECMAScript's laxness around thenables. As proposed in this repository, Observables are not part of the ECMAScript language, so they don't get special ES<->WebIDL conversion steps that we might be able to use to accept anything that:

  • Is an Observable; or
  • Is a Promise<T>; or
  • Has a non-null Symbol.iterator; or
  • Has a non-null Symbol.asyncIterator protocol

Given that, I was at least hoping we'd be able to supply some kind of nifty conversion algorithm to convert these kinds of types to native Observables, that WebIDL itself could hook into so that any web platform methods that want to accept an Observable can just accept the Observable type itself (and the conversion will be done automatically as part of the bindings). Unfortunately I don't think that's possible, nor does it seem possible to express an async iterable type in WebIDL either. For example, whatwg/webidl#874 seems to indicate that the best we can do is accept an any and then do the conversion manually by calling ES's GetIterator(object, async), as the Streams Standard does here: https://streams.spec.whatwg.org/#readable-stream-from-iterable.

This isn't horrible, and it doesn't seem like a serious blocker or anything, but if Observables became widely used on the platform we'd be encouraging a bunch of any methods that have to delegate to some abstract conversion algorithm that our spec will add. We can always introduce syntax to WebIDL later to make it possible to express a type that is async iterable (something like async sequence<T> perhaps); this would allow us to use methods that accept a typedef (Promise<T> or sequence<T> or async sequence<T>) ObservableInit — except oh wait, Promises can't be used in unions! So I guess we're stuck with encouraging any...

README.md Outdated Show resolved Hide resolved
@domfarolino domfarolino marked this pull request as ready for review September 7, 2023 17:22
@bakkot
Copy link
Contributor

bakkot commented Sep 7, 2023

Note that in principle something can have both a then method and a Symbol.iterator method, so if you're checking for both, you have to specify which order they're checked.

Edit: just saw this was already pointed out above, sorry for the noise.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@domenic
Copy link
Collaborator

domenic commented Sep 7, 2023

The big blocker on the Web IDL level is accepting Promise<T>s overloaded with anything else. That's explicitly discouraged for most APIs, because promises are special and we want people to be able to pass x instead of requiring them to wrap in Promise.resolve(x). However, observables are special too, so it makes sense we'd need an exception here.

My suggestion is that in the spec, you define a conversion operation from a JS value to a Web IDL Observable value, using JS spec-type prose. (Like Web IDL does for its conversion algorithms: example.) The first line of any algorithm that takes an "observable-like" should be

  1. Set observableArg to the result of [converting to an Observable] given observableArg.

and then the algorithm can proceed from there assuming observableArg is a Web IDL Observable. This quarantines off the JS spec-type prose and value handling into one algorithm, which echoes how Web IDL tries to quarantine off that stuff from the rest of the platform's specs. (Or how the binding layer in implementations quarantines off dealing with the JS engine stuff from the rest of the implementation.)

In the future we can add some sort of ObservableLike type to Web IDL which does this step for you. In the meantime, writing the spec this way will set you up for such a nice future.

@annevk
Copy link

annevk commented Sep 8, 2023

I suspect we'll want to use that the moment something wants to consume an Observable? Which I guess starts with streams? And would we ever want to distinguish between ObservableLike and Observable? Probably not.

@domfarolino
Copy link
Collaborator Author

My suggestion is that in the spec, you define a conversion operation from a JS value to a Web IDL Observable value, using JS spec-type prose.

Yep, that was my plan, I just wasn't sure if that (plus forcing Observable consumers to accept any) was a bad practice or felt too hacky, but I think it is fine since at the very least there's precedent for Streams doing it for async iterables. So I think we're "unblocked" here, but I do wonder what the long-term state of affairs might look like. There are two possible follow-ups I can imagine, that seem sort of orthogonal to each other:

  • Making Web IDL allow Promise<T> in unions & give Web IDL a syntax for async iterables: this lets Observable consumers express arguments in terms of ObservableLike which is a union of a bunch of types. This has some potential downsides I'm not sure of yet:
    • Again, what if a type implements multiple of the characteristics described by the union? Does the union conversion "prioritize" conversion order somehow? My understanding is that the spec user is responsible for detecting the active field of the union, like https://fetch.spec.whatwg.org/#concept-bodyinit-extract does, however https://webidl.spec.whatwg.org/#es-union seems to do some preprocessing itself, and eagerly recognizes sequence types, grabbing the @@iterator, which I'm not sure we actually want if we're prioritizing async iterables over iterables etc.
  • Making the Observable type a special Web IDL type that automatically does the conversion we seek for us, but this seems tricky because then it would not only convert things -> Observables for arguments, but would do the same for return values. This is something that I think @annevk mentioned should be avoided in https://matrixlogs.bakkot.com/WHATWG/2023-09-08. I'm not sure how we'd get automatic conversions for Observable arguments, but not Observable return values though.

And would we ever want to distinguish between ObservableLike and Observable? Probably not.

The only thing I can think of is the last point I mentioned above, which is that we'd probably want to allow various things to be passed in as (and converted to) an Observable, while preventing code from returning i.e., async iterables in place of an expected Observable return value. At least that's what I thought you were saying on Matrix (and kind of how I interpreted the discussion in #44).

@annevk
Copy link

annevk commented Sep 8, 2023

I think you misunderstood me. Returning Observable is reasonable. The problem is with returning "async sequence" and what that might mean. Making Observable a special type like Promise is probably the best therefore, on reflection. ES-to-IDL would be involved and have "the magic". And IDL-to-ES would be essentially just returning a reference to the JS wrapper object.

This is assuming we'd have many more APIs that want to consume Observable. And that generally we'd want to either return a stream type or Observable and not some other "async sequence".

@domfarolino
Copy link
Collaborator Author

Got it. I've clarified offline that @annevk was talking about whether or not we actually wanted to expose "async sequence" directly as a Web IDL type, and was not concerned about the automatic "async iterable => Observable" conversion incurred when a developer returns an async iterable from a function whose IDL callback return type is an Observable.

I'll update the PR to reflect the option of making Observables its own WebIDL type that'd automatically perform the kind of conversion discussed above when going from ES->IDL.

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 this pull request may close these issues.

Iterator.from() seems to be a thing now
4 participants