Skip to content

Conversation

@dead-claudia
Copy link

@dead-claudia dead-claudia commented Aug 16, 2018

Fixes: #197
Fixes: #196
Fixes: #186
Fixes: #184
Fixes: #176
Fixes: #165
Fixes: #91

This doesn't yet include updated tests, but I did update the spec.

I purposefully tried to leave out spec changes/fixes that might be controversial, like changing/removing the 3-arg subscribe variant or not swallowing errors. But here's a list of what I did fix (I think this is all):

  • I fixed a slew of erroneous non-throwing assertions.
  • I fixed a few typos, formatting errors, and other related mistakes and inconsistencies.
  • I fixed Observable.from to correctly use @@iterator for the iterable case.
  • I edited the "HostReportErrors" stuff to just use a common shorthand instead, so it's a bit more readable and intelligible, and to fix a few inconsistencies. Implementations are likely to just use a macro or do what polyfill writers do and desugar them into try/catch blocks.
  • I merged the next/error/complete operations to just use variants of the same core algorithm. I also used it to replace the erroneous object method call within Observable.prototype.subscribe. This simplified the spec tremendously without loss of clarity.
  • I added quite a few spec assertions over internal slots and their types, for clarity.
  • I fixed the type checks that went "if O does not have the [[Foo]] internal slot" to read "if O does not have all of the internal slots of Bar objects" instead, for consistency and clarity.
  • I fixed all the subscription observer callbacks for Observable.from and Observable.of to 1. type-check their input, and 2. work on their arguments generically instead of assuming they're subscription observers.
  • I added @@toStringTag support for the various types (observable, subscription, and subscription observer).
  • I elected to defer the unsubscribe method check instead of accessing it twice. I still maintained the object check so engines can avoid handling nonsensical cases.
  • I changed all the internal slot accesses to match the mainline spec - it's way more concise.
  • I fixed a lot of consistency issues within the README and clarified a few things with (usually) added annotations. I also changed it to use TypeScript highlighting, so it's less likely to be odd and unusual in editors.

I'm aware it reads a bit like shotgun surgery, but when I was trying to write an optimized implementation of this (with JIT-compiled pipelines), trying to figure out the behavior I needed to maintain required so many explicit spec deviations I decided my time was better spent fixing the spec so I could do the experiment. And yes, a lot of things needed fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment