[fixed] rewrite waiting for an emission from every stream before working
Needs ReviewPublic

Authored by appsforartists on Wed, Sep 6, 8:28 PM.

Details

Summary

Formerly, switchMe$.rewrite({ a: a$, b: b$ }) wouldn't dispatch anything until both a$ and b$ had dispatched, event if switchMe$ had only dispatched a. This resolves that issue.

The solution was to add an option to _reactiveNextOperator and combineLatest to determine whether or not they should wait until all children have dispatched before themselves dispatching.

In order to add an option to _reactiveNextOperator, its signature needed to change. Instead of accepting its arguments as positional arguments, it now accepts them in a single list. This is consistent with our other APIs like isAnyOf. To support strong typing, it still passes values into its operation positionally. ... must come at the end of a function signature, but not a function call. Thus, this shouldn't cause any issues.

Normally for a change of this magnitude, I'd write a codemod, but since I don't believe anyone but me has written code against these APIs, I've made the changes by hand in my own projects.

There's a bigger API design question (should we use named args everywhere, or pass options dictionaries to APIs like _reactiveNextOperator?) captured in https://github.com/material-motion/material-motion-js/issues/193#issuecomment-327636972

Diff Detail

Repository
R13 material-motion/material-motion-js
Branch
swipe-to-dismiss (branched from develop)
Lint
Lint OK
Unit
No Unit Test Coverage
appsforartists created this revision.Wed, Sep 6, 8:28 PM
Restricted Application added a reviewer: O2: Material Motion. · View Herald TranscriptWed, Sep 6, 8:28 PM
Restricted Application added a project: Material Motion. · View Herald Transcript
Restricted Application added a reviewer: O3: Material JavaScript platform reviewers. · View Herald Transcript
Restricted Application added a reviewer: Material Motion. · View Herald Transcript
Restricted Application failed to build Restricted Buildable!Wed, Sep 6, 8:31 PM
Restricted Application completed building Restricted Buildable.Wed, Sep 6, 8:41 PM
featherless added inline comments.
packages/core/src/operators/scaledBy.ts
37

Why must these all be arrays now?

appsforartists added inline comments.Tue, Sep 12, 6:01 PM
packages/core/src/operators/scaledBy.ts
37

TypeScript doesn't let you have a variable number of arguments before a known argument. This is not a valid signature:

function reactiveMap(transform, ...streams, options)

Removing the ... makes streams into an Array, so I can write this signature:

function reactiveMap(transform, streams, options)

So, the short answer is that I needed to make streams a fixed length so I could add options as the final parameter. There's a bigger discussion to be had as to whether options should be the last parameter, or if every operator should take a single dictionary argument (which could contain individual options). That discussion is in https://github.com/material-motion/material-motion-js/issues/193#issuecomment-327636972.