WICG / observable

Observable API proposal
https://wicg.github.io/observable/
Other
543 stars 12 forks source link

Spec: `forEach()` does nothing with "visitor callback controller" AbortController #108

Closed domfarolino closed 4 months ago

domfarolino commented 4 months ago

The "visitor callback controller" is never used to trigger the abort of any signal, I think. So something is wrong here and I need to take a closer look.

domfarolino commented 4 months ago

OK so it turns out that "visitor callback controller" is actually used, so this is a non-issue. It just looks like it is not used because bikeshed won't highlight all of the references to it inside the algorithm. That's really weird. @tabatkins do you know why this might be? If you go to https://wicg.github.io/observable/#:~:text=and%20signal%20abort-,visitor%20callback%20controller,-with%20E. and click that variable, notice that the instance of that variable in step 2 of that whole algorithm/div does not highlight...

tabatkins commented 4 months ago

The immediate cause is that the final instance of the term is broken across the line in the source, so Bikeshed isn't recognizing it as the same term as the other two instances. But that's probably broken behavior; I should be normalizing whitespace. I wonder if that's a regression from my changes a few months ago?

domfarolino commented 4 months ago

Oh interesting. Yeah I guess I'd expect that to still work? At least a lot of WHATWG standards are happy to break in the middle of variable names, so I'd expect bs to handle that. If you could see if that's a recent regression that would be great, or if I can help let me know.

tabatkins commented 4 months ago

Ah, yes, I was once again fooled by JS's replace function:

function nameFromVar(el) {
    return el.textContent.replace(/(\s|\xa0)+/, " ").trim();
}

That needs to be a global regex. :/

domfarolino commented 4 months ago

Ah, nice find! OK I'll close this, and I assume bikeshed will get fixed at some point in the meantime? Thanks for the quick investigation!