Repository: vigetlabs/microcosm
Pull request #516 (2018-05-08 12:52:00)

[microcosm] Always, synchronously, emit the first observer.next()
This PR updates Observable such that the first "next" callback invokes immediately. This is important for Observable::map, particularly when binding data in React. Otherwise, React renders before the scheduler clears the queue for an observable. So you'll end up with something like:


```javascript
class UserShow extends Presenter {
  getModel(repo, { id }) {
    let { users } = repo.domains

    return {
      user: users.map(users => find(users, { id })
    }
  }
  render() {
    console.log(this.model.user) 
    // Always undefined initially because the observable callback is still
    // in the scheduler
  }
}
```

Essentially, this makes it so that you can always get a value immediately, even if the initial payload is empty.

codecov-io: # [Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/516?src=pr&el=h1) Report
> Merging [#516](https://codecov.io/gh/vigetlabs/microcosm/pull/516?src=pr&el=desc) into [master](https://codecov.io/gh/vigetlabs/microcosm/commit/7c4c0d7e45a3818b60177184b3c9f14fd11b31e7?src=pr&el=desc) will **decrease** coverage by `0.25%`.
> The diff coverage is `90%`.

[![Impacted file tree graph](https://codecov.io/gh/vigetlabs/microcosm/pull/516/graphs/tree.svg?width=650&height=150&src=pr&token=AaE0FLz9sU)](https://codecov.io/gh/vigetlabs/microcosm/pull/516?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #516      +/-   ##
==========================================
- Coverage   98.78%   98.52%   -0.26%     
==========================================
  Files          30       30              
  Lines         738      748      +10     
  Branches      147      150       +3     
==========================================
+ Hits          729      737       +8     
- Misses          9       11       +2
```


| [Impacted Files](https://codecov.io/gh/vigetlabs/microcosm/pull/516?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/microcosm/src/domain.js](https://codecov.io/gh/vigetlabs/microcosm/pull/516/diff?src=pr&el=tree#diff-cGFja2FnZXMvbWljcm9jb3NtL3NyYy9kb21haW4uanM=) | `100% <100%> (ø)` | :arrow_up: |
| [packages/microcosm/src/observable.js](https://codecov.io/gh/vigetlabs/microcosm/pull/516/diff?src=pr&el=tree#diff-cGFja2FnZXMvbWljcm9jb3NtL3NyYy9vYnNlcnZhYmxlLmpz) | `94.54% <88.88%> (-1.54%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/516?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/516?src=pr&el=footer). Last update [7c4c0d7...81fa97a](https://codecov.io/gh/vigetlabs/microcosm/pull/516?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

efatsi: 👍 thanks for all the inline comments on the more obscure things
nhunzaker: Thanks, guys! @zporter I've added a note about that case, as you requested.

Patch:
diff --git a/packages/microcosm/src/domain.js b/packages/microcosm/src/domain.js
index 1cfd6ccf..c8af1237 100644
--- a/packages/microcosm/src/domain.js
+++ b/packages/microcosm/src/domain.js
@@ -71,6 +71,13 @@ export class Domain<State: any = Object> extends Agent {
   }
 
   receive(action: Subject): void {
+    // Avoid a situation where history dispatches before a domain
+    // is fully set up. Domains move into the "next" state when
+    // construction is finished (getInitialState())
+    if (this.status === 'start') {
+      return
+    }
+
     let next = this._rollforward(action)
 
     if (next !== this.payload) {
diff --git a/packages/microcosm/src/observable.js b/packages/microcosm/src/observable.js
index 3094bcfd..51496845 100644
--- a/packages/microcosm/src/observable.js
+++ b/packages/microcosm/src/observable.js
@@ -9,14 +9,16 @@ import { noop } from './empty'
 import { scheduler } from './scheduler'
 import { isObject } from './type-checks'
 
+// A subscription is constructing
+const START = 0
 // A subscription has no work in progress
-const IDLE = 0
+const IDLE = 1
 // A subscription will send, but the scheduler is not ready for it.
-const BUFFERING = 1
+const BUFFERING = 2
 // A subscription is processing
-const WORKING = 2
+const WORKING = 3
 // A subscription is done
-const CLOSED = 3
+const CLOSED = 4
 
 const NEXT = 'next'
 const ERROR = 'error'
@@ -82,7 +84,7 @@ export class Subscription implements Unsubscribable {
   _queue: Queue
 
   constructor(observer: Observer, subscriber: Subscriber, origin: *) {
-    this._state = IDLE
+    this._state = START
     this._cleanup = noop
     this._queue = []
     this._observer = observer
@@ -94,9 +96,21 @@ export class Subscription implements Unsubscribable {
       this._cleanup = subscriber.call(origin, subscriptionObserver) || noop
     } catch (error) {
       subscriptionObserver.error(error)
+
+      // If an exception is raised in a subscriber and there isn't an
+      // error handler, we raise with the scheduler. This happens for
+      // cases like:
+      //
+      // let obs = new Observable(observer => { throw "Crud" })
+      //
+      // obs.subscribe(next => {}) // Raises an exception with the scheduler
+      // obs.subscribe(next => {}, error => {}) // Doesn't raise
+      if (observer.error === noop) {
+        scheduler().raise(error)
+      }
     }
 
-    if (this._state === IDLE) {
+    if (this._state === IDLE || this._state === START) {
       this._state = WORKING
     }
   }
@@ -183,7 +197,6 @@ function cleanupSubscription(subscription: Subscription): void {
   // Drop the reference to the cleanup function so that we won't call it
   // more than once
   subscription._cleanup = noop
-
   if (typeof cleanup === 'function') {
     cleanup()
   } else if (cleanup instanceof Subscription) {
@@ -252,18 +265,39 @@ function onNotify(subscription: Subscription, type: string, value: *) {
   }
 
   switch (subscription._state) {
-    case CLOSED:
-      // If closed, do nothing
-      break
-    case BUFFERING:
-      // Until the scheduler is ready to send out an update, push
-      // notifications into a backlog
-      subscription._queue.push({ type, value })
-      break
-    case WORKING:
-      // Go ahead and send messages that result from subscription
-      // callbacks while the subscription is processing
-      notifySubscription(subscription, type, value)
+    case START:
+      // When a subscription starts, immediately fire off the first "next"
+      // callback. This allows features like *.map to immediately emit a
+      // value, which is particularly useful for UI use cases where you
+      // want to immediate pluck a value out of a Domain.
+
+      // No matter what happens, always move into the idle state. All future
+      // updates should be queued asynchronously
+      subscription._state = IDLE
+
+      if (type === NEXT) {
+        // This state occurs if a consumer calls "observer.next" synchronously:
+        //
+        //   new Observable(observer => {
+        //     observer.next(true)
+        //   })
+        notifySubscription(subscription, type, value)
+      } else {
+        // Otherwise, a subscriber closed immediately. We absolutely know that
+        // there will be no other updates
+        //
+        // This state occurs if a consumer calls "observer.complete"
+        // or "observer.error" without any other update:
+        //
+        //   new Observable(observer => {
+        //     observer.complete()
+        //   })
+        //
+        // These must be asynchronous to give the cleanup method space to execute
+        scheduler().push(
+          notifySubscription.bind(null, subscription, type, value)
+        )
+      }
       break
     case IDLE:
       // For any other phase, we are now buffering. Initiate a new
@@ -277,13 +311,25 @@ function onNotify(subscription: Subscription, type: string, value: *) {
 
       subscription._state = BUFFERING
       subscription._queue.push({ type, value })
+
       scheduler().push(flushSubscription.bind(undefined, subscription))
       break
+    case BUFFERING:
+      // Until the scheduler is ready to send out an update, push
+      // notifications into a backlog.
+      subscription._queue.push({ type, value })
+      break
+    case WORKING:
+      // Go ahead and send messages that result from subscription
+      // callbacks while the subscription is processing.
+      notifySubscription(subscription, type, value)
+      break
+    case CLOSED:
+      // If closed, do nothing.
+      break
     default:
-      // There are two cases for which we can get into this state:
-      //   1. A subscription is closed
-      //   2. We've made a mistake and assigned an invalid state.
-      // Both are exceptions. Throw in strict mode
+      // The only way this could occur is if a consumer of Microcosm somehow
+      // messes with the Observable state code. This is a hard failure.
       assert(
         false,
         `Observable subscription is in an invalid state.` +


Test patch:
diff --git a/packages/microcosm/test/unit/domain/register.test.js b/packages/microcosm/test/unit/domain/register.test.js
index bb876a8a..2278a5e5 100644
--- a/packages/microcosm/test/unit/domain/register.test.js
+++ b/packages/microcosm/test/unit/domain/register.test.js
@@ -63,4 +63,38 @@ describe('Domain::register', function() {
 
     expect(repo.state.test).toBe('test')
   })
+
+  it('should have state before an action resolves', async () => {
+    expect.assertions(2)
+
+    let repo = new Microcosm()
+    let action = n => Promise.resolve(n)
+
+    repo.addDomain('test', {
+      getInitialState() {
+        return 'start'
+      },
+      register() {
+        return {
+          [action]: {
+            next: (a, b) => `next: ${b}`,
+            complete: (a, b) => `complete: ${b}`
+          }
+        }
+      }
+    })
+
+    let job = repo.push(action, 2)
+
+    job.subscribe({
+      next() {
+        expect(repo.domains.test.payload).toBe('next: 2')
+      },
+      complete() {
+        expect(repo.domains.test.payload).toBe('complete: 2')
+      }
+    })
+
+    await job
+  })
 })
diff --git a/packages/microcosm/test/unit/subject-map.test.js b/packages/microcosm/test/unit/subject-map.test.js
index bc20c357..79b55127 100644
--- a/packages/microcosm/test/unit/subject-map.test.js
+++ b/packages/microcosm/test/unit/subject-map.test.js
@@ -199,7 +199,7 @@ describe('SubjectMap', function() {
       })
 
       expect(hash).toHaveProperty('payload.now', 1)
-      expect(hash).toHaveProperty('payload.later', undefined)
+      expect(hash).toHaveProperty('payload.later', 1)
 
       await hash
 
diff --git a/packages/microcosm/test/unit/subject.test.js b/packages/microcosm/test/unit/subject.test.js
index fe6b5b5c..fb1fc07d 100644
--- a/packages/microcosm/test/unit/subject.test.js
+++ b/packages/microcosm/test/unit/subject.test.js
@@ -240,26 +240,17 @@ describe('Subject', function() {
     })
   })
 
-  describe('toString', function() {
-    it('stringifies to its key name', () => {
-      expect(new Subject(null, { key: 'foobar' }).toString()).toBe('foobar')
-    })
-
-    it('stringifies to "subject" when given no key', () => {
-      expect(new Subject(null).toString()).toBe('subject')
-    })
-  })
-
   describe('toJSON', function() {
     it('generates a POJO', () => {
-      let subject = new Subject(true, { key: 'foobar' })
+      let subject = new Subject()
 
+      subject.next(true)
       subject.complete()
 
       expect(subject.toJSON()).toEqual({
+        key: 'subject',
         payload: true,
-        status: 'complete',
-        key: 'foobar'
+        status: 'complete'
       })
     })
   })
@@ -292,20 +283,17 @@ describe('Subject', function() {
       expect(answers).toEqual([0, 2, 4, 6, 8])
     })
 
-    it('starts with the current value', async () => {
+    it('starts with the current value', () => {
       let subject = new Subject()
 
       let double = subject.map(value => value * 2)
       let answers = []
 
       subject.next(5)
-
       double.subscribe(value => answers.push(value))
-
       subject.complete()
 
-      await subject
-
+      // Mapping over a subject should be immediate
       expect(answers).toEqual([10])
     })