Editorial review: Document the Observable API #39237
Conversation
chrisdavidmills
requested review from
pepelsbey and
wbamberg
and removed request for
a team
chrisdavidmills
changed the title
|
@chrisdavidmills, Is this ready for review? @benlesh are you able to take a look? If not (if we don't hear back) I will take a look if Chris says it is ready. |
Yes! I'd love a review of what I've done so far, to make sure I'm on the right track. |
|
One general piece of feedback would be that I notice a lot of sentences either start with, or contain "Observable object instance(s)", and I would just change all of these to "Observables", since I think that reads a lot smoother, and is less wordy. |
@domfarolino yup, good call. I think I've caught all of these. |
|
Thanks a lot for the feedback so far, @domfarolino. I will start adding the reference pages now as well, so you'll have those to review soon too. |
|
Anything blocking this PR? |
|
@Josh-Cena nobody at Google seems to be available to finish reviewing it. I'll ask @rachelandrew to chase it up again. |
|
I'm happy to do any kind of review (technical/editorial) to get it merged; just lmk. This was a tc39 proposal so I do have some knowledge about it. |
Thanks @Josh-Cena, that would be super helpful. I've looked back through my email threads on the subject and found the following description of what is still missing:
Can you help me with the |
|
@Josh-Cena . OK, I've added a |
|
That's totally fair. I find difficulties coming up with examples for half of the pages I write as well. For starters, perhaps we can create a few dummy observables and demonstrate how they can be composed, in the style of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Iterator/flatMap. As for
So, it's like |
domfarolino
left a comment
domfarolino
left a comment
There was a problem hiding this comment.
This is lookin good! Just a few more comments
|
|
||
| ### Basic `catch()` example | ||
|
|
||
| TBD |
There was a problem hiding this comment.
Is this going to be filled out here or in a follow-up?
There was a problem hiding this comment.
I don't understand how to write a simple and obvious example that would make use of this. Same for the switchMap page that I just added. In the interests of getting this landed, can you write me some simple snippets, then we can look to add some better examples later on?
There was a problem hiding this comment.
Generally, this is going to be useful when you wrap something that could fail.
A basic JSON formatter using catch:
<textarea id="json-entry-textarea"></textarea>
<pre id="output-area"></pre>const textarea = document.getElementById('json-entry-textarea');
const outputArea = document.getElementById('output-area');
function timeout(ms) {
return new Observable((subscriber) => {
const id = setTimeout(() => {
subscriber.next()
subscriber.complete()
})
subscriber.addTeardown(() => clearTimeout(id))
})
}
textarea.when('keypress')
.switchMap(() => {
const text = textarea.value
// basic throttle
return timeout(1000).map(() => {
// this could fail.
return JSON.parse(text))
})
.catch((error) => {
// gracefully handle the error, so we
// don't stop listening for textarea keypresses
console.error(error);
})
})
.subscribe(obj => {
outputArea.textContent = JSON.stringify(obj, null, 2)
})A "retry" example using catch:
<div id="lightbox" style="width:50px; height:50px; background-color:black;"></div>function streamingFetch(url) {
return new Observable(async (subscriber) => {
try {
const request = await fetch(url, { signal: subscriber.signal });
if (!request.ok) {
throw new Error(`Request ${request.status}`);
}
const reader = await request.getReader();
const decoder - new TextDecoder();
for await (const buffer of reader) {
subscriber.next(decoder.decode(buffer));
}
subscriber.complete();
} catch (err) {
subscriber.error(err);
}
})
}
const streamingOnOffCommands = streamingFetch('/streaming/on-off-signals')
let endlessStreamingOnOffCommands
endlessStreamingOnOffCommands = streamingOnOffCommands.catch((error) => {
console.error(error);
return endlessStreamingOnOffCommands;
})
const lightbox = document.getElementById('lightbox');
endlessStreamingOnOffCommands.subscribe((command) => {
lightbox.style.backgroundColor = command === 'on' ? 'yellow' : 'black';
})There was a problem hiding this comment.
These of course, are just ideas. And I didn't test the code. haha.
There was a problem hiding this comment.
Oh: The real thing is catch is MOST useful when handling errors inside of a switchMap or a flatMap (or even another catch). This is done usually to guard the outer subscription from being terminated by an error merged in from an inner subscription.
Otherwise, you can just use it like syntactic candy:
button.when('click')
.map((value, i) => {
if (Math.random() > 0.5) throw new Error('oops!');
return i;
})
.catch(error => {
console.error(error)
return 'ERROR'
})
.subscribe(console.log);Rather than:
button.when('click')
.map((value, i) => {
if (Math.random() > 0.5) throw new Error('oops!')
return i;
})
.subscribe({
next: console.log,
error: (error) => {
console.error(error)
console.log('ERROR')
}
});Although those examples, obviously a catch in the mapping function is probably more appropriate, but you get the idea.
someObservable.catch(errorHandler).subscribe(successHandler)
Also, since observables are reusable, you can "bake-in" error handling using catch for all consumers in this way, even if you didn't "own" the original observable creation. (the "retry" example shows that a bit)
There was a problem hiding this comment.
OK, so I got the basic syntactic sugar catch() example working, kind of. I ended up modifying it slightly to:
const btn = document.querySelector("button");
btn
.when("click")
.map((value, i) => {
if (Math.random() > 0.5) throw new Error("oops!");
return i;
})
.catch((error) => {
console.error(error);
return "ERROR";
})
.subscribe(console.log);When the button is pressed, it console.logs() the index numbers. However, when the error fires, I get the following TypeError:
Uncaught TypeError: Failed to execute 'catch' on 'Observable': Cannot convert value to an Observable. Input value must be an Observable, async iterable, iterable, or Promise.
This is why I'm finding observables catch() so confusing. You can't just get it to throw an error when things go wrong. You have to pass it a callback that returns something that converts to an observable.
There was a problem hiding this comment.
I also tried to get the JSON formatter example working, as it seems like the better candidate for a self-contained example for an MDN page, but it doesn't work. Some thoughts:
- The
setTimeout()'s callback needs to have themsargument included inside it? - The
subscriber.next()function needs to have a value passed to it. Currently the code fails withUncaught TypeError: Failed to execute 'next' on 'Subscriber': 1 argument required, but only 0 present.But I'm really not sure what that should be. I'm thinking it should be the<textarea>'stextContent, but that's handled inside theswitchMap()...
|
|
||
| ## Examples | ||
|
|
||
| ### Basic `from()` example |
There was a problem hiding this comment.
It would be nice to have a more complicated example with AsyncIterator, or something else, too. This can come as a follow-up though!
There was a problem hiding this comment.
Again, I'm not sure how to do that. I'd vote for deferring that so we can get this stuff landed.
There was a problem hiding this comment.
Spit balling, I'd say just use any API that exposes one. Readers are the only one I can think of off the top of my head, but I'm not sure if that works across all browsers yet.
const request = await fetch('url')
const reader = await fetch.getReader()
const decoder = new TextDecoder()
const stream = Observable.from(reader).map((buffer) => decoder.decode(buffer))There was a problem hiding this comment.
I'm just gonna leave this one for now.
Thanks. I've read through the WICG thread, and I conceptually get what it is supposed to be doing, but I don't get how to write an observables version of that. I've just tried playing with it for an hour or so, and I still can't figure out a simple and useful switchMap example. I keep writing little examples and finding that flatMap is really what I want, not switchMap. |
Josh-Cena
requested review from
Josh-Cena
and removed request for
domfarolino and
pepelsbey
| } | ||
| }, 500); | ||
| subscriber.addTeardown(() => { | ||
| countBtn.textContent = "Restart count"; |
There was a problem hiding this comment.
Instead of the subscriber.active check above. Just call clearInterval(interval) here. It will automatically execute when the controller.abort() is called below. (Or also if there's a complete or error, but that doesn't happen in this code example)
There was a problem hiding this comment.
OK, cool, that seems to work. Updated. I've also updated the explanation below.
| outputElem.textContent = "Count complete"; | ||
| }, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This is going to start a new interval every time the button is clicked... Here's a slightly improved example:
const outputElem = document.querySelector("p");
const btn = document.querySelector("button");
function interval(ms, count) {
return new Observable((subscriber) => {
let n = 1;
const interval = setInterval(() => {
subscriber.next(n++)
if (n > count) {
subscriber.complete()
}
})
subscriber.addTeardown(() => clearInterval(interval))
})
}
let controller
btn.addEventListener('click', () => {
// cancel the previous count
controller?.abort()
controller = new AbortController()
if (btn.textContent === "Start count") {
btn.textContent = "Restart count";
}
interval(500, 10).subscribe({
next: (value) => {
outputElem.textContent = value;
},
complete: () => {
outputElem.textContent = "Count complete";
},
}, { signal: controller.signal })
})or even more concisely using observables:
btn.when('click')
.inspect({ next: () => {
// on the first click, we change the text.
if (btn.textContent === "Start count") {
btn.textContent = "Restart count";
}
})
// on each click, start a new interval and
// cancel the previous one
.switchMap(() => interval(500, 10))
.subscribe({
next: (value) => {
outputElem.textContent = value;
},
complete: () => {
outputElem.textContent = "Count complete";
},
})There was a problem hiding this comment.
A couple of things:
- I deliberately disable the count button when the interval is active, so multiple intervals are not created.
- Your examples look more succinct and efficient than my approach, but I'm not convinced they are more understandable. They also don't offer the same functionality that I wanted to provide (e.g. being able to abort manually using an abort button)
- As written, neither of them seem to work.
| i++; | ||
| }, 500); | ||
|
|
||
| subscriber.signal.addEventListener("abort", () => { |
There was a problem hiding this comment.
generally speaking, no one should really use .signal.addEventListener. subscriber.addTeardown is a much better API for that. they're both similar, but the latter will ensure that the teardown is called even if the subscriber was already inactive by the point of the registry. .signal.addEventListener is a footgun in that regard.
There was a problem hiding this comment.
Good call, thanks for this! I've updated the code and the explanation as required.
|
Hi @benlesh @domfarolino bumping this. I'll proceed with editorial review when either of you gives a +1. |
chrisdavidmills
changed the title
Thanks for the ping, @Josh-Cena. I've decided to move this to the editorial review stage, as the technical review stage seems to be completely stalled. How about you start reviewing this stuff slowly but steadily, and I'll make updates after you review each part, to keep it manageable? When we run into the bits that are still incomplete, we can ask @domfarolino and @benlesh for input on individual, specific items, which might be less overwhelming and lead to better results. |
5 participants
Description
This PR documents the Observable API, which is available in Chrome 135 onwards — see the relevant ChromeStaus entry here: https://chromestatus.com/feature/5154593776599040.
Specifically, this PR adds:
EventTarget.when()referenceObservable()referenceSubscriberreferenceMotivation
Additional details
Related issues and pull requests