[refactored] pluck to use named args
ClosedPublic

Authored by appsforartists on Oct 6 2017, 11:53 PM.

Diff Detail

Repository
R13 material-motion/material-motion-js
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
appsforartists created this revision.Oct 6 2017, 11:53 PM
Restricted Application added a reviewer: O2: Material Motion. · View Herald TranscriptOct 6 2017, 11:54 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 completed building Restricted Buildable.Oct 6 2017, 11:57 PM
vietanh added inline comments.
packages/core/src/operators/pluck.ts
35

Can you explain why you chose 'path' over 'key' here?

appsforartists added inline comments.Oct 10 2017, 8:19 PM
packages/core/src/operators/pluck.ts
35

If you give pluck a path, it will walk that path in a nested object. For instance, in this totally contrived example:

{
  style: {
    transform: {
      translate: {
        x: 50,
        y: 10,
      }
    }
  }
}

pluck('style.transform.translate') would emit { x: 50, y: 10 }. In most cases, you're only plucking one level deep, so either key or path would be appropriate. I used path because it's a more precise name for multi-level-deep plucks.

vietanh accepted this revision.Oct 10 2017, 8:27 PM
vietanh added inline comments.
packages/core/src/operators/pluck.ts
35

Got it, then it would be hugely helpful for future readers if you could include example such as pluck('style.transform.translate') in this comment here.

This revision is now accepted and ready to land.Oct 10 2017, 8:27 PM
appsforartists added inline comments.Oct 10 2017, 8:33 PM
packages/core/src/operators/pluck.ts
35

That's the intent of line 43:

* - `transform$.pluck({ path: 'translate.x' })` is equivalent to
*   `transform$.map(transform => transform.translate.x)`
This revision was automatically updated to reflect the committed changes.