add flattenProps

[alex-wilmer]

found a need for this when a relay container has multiple fragments.

this would amplify the possibility of prop name collisions, but not any more so than calling flattenProp twice.

[codecov-io]

Codecov Report

Merging #366 into master will increase coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master     #366     +/-   ##
=========================================
+ Coverage   92.98%   93.09%   +0.1%     
=========================================
  Files          51       52      +1     
  Lines         328      333      +5     
=========================================
+ Hits          305      310      +5     
  Misses         23       23

Impacted Files	Coverage Δ
src/packages/recompose/flattenProps.js	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82c8287...0c2a402. Read the comment docs.

[wuct]

Thanks! Looks good to me.

[alex-wilmer]

@wuct I'm not seeing the same prettier error locally. any ideas?

Followprettier formatting (expected '\n' but found ' ') prettier/prettier

[alex-wilmer]

okay this should do it. prettier just was updated between PR

[istarkov]

About prettier - you need to rebase to see an errors.

Wanna add what I think about.
I understand that if we have renameProps why not to have flattenProps. But I highly dislike the idea of flattenProp itself - I see it no better than ordinary withProps

withProps(({ blabla }) => ({...blabla}));

Yes it somehow longer, but not a lot. Having that js world is moving into direction of typed languages we see that type of withProps operations can be inferred based on ES6 syntax, and flattenProp('blabla') is some kind of dynamic construct (even flow can recognise this as I think)

It's just IMO but all methods except withPropsOnChange (it result type is independent of array arg) which used strings for transforms will be deprecated in near future.

[alex-wilmer]

/shrug I agree, not a big fan of strings in general for the same reason.

I'll leave this open and let you guys decide, not a blocker for me of course.

[alex-wilmer]

Also I assume you mean just passing the object. Or is spreading into an empty object somehow better for type inference?

withProps(({ blabla }) => blabla)

[istarkov]

Your solution is better for one prop

[alex-wilmer]

OK well for the sake of argument.. if strings are still 'good enough' not to be deprecated, perhaps a touch of lodash style pathing would make this even more useful?

let props = {foo: {nested: { a, b }}}
compose(
  flattenProp('foo.nested')
)({ foo, nested, a, b }) => <C />))

[alex-wilmer]

Since destructing still kind of sucks in the crashing null errors, something that _.get() solves nicely, I think that would be a good addition. thoughts?

[alex-wilmer]

Could even pass a default like get

let props = {foo: {nested: { a, b }}}
compose(
  flattenProp('foo.nested', { a: 1, b: 2 })
)({ foo, nested, a, b }) => <C />))

[istarkov]

IMO lodash was created in a time where we had no ES6 language,
I think now all that flatten pick etc is not so useful,
without function keyword, having destructuring etc it's easy now to write almost anything without.

Undefined errors are now solved with flowtype, typescript so not a problem, yes syntax still sometimes looks ugly like ({ blabla, blublu } = {}) => fn(blabla, blublu)

[alex-wilmer]

types don't help you with runtime api response though, like relay data. you either have to write tons of defaults like ({ foo: { nested: { a, b } = {} } = {} } = {}) or use a utility

^ that's a bad example, but you know what i mean. responses can be null when you're trying to access nested args

[alex-wilmer]

In any case.. I'm playing devil's advocate. I'm not sure this would belong in recompose proper anyways

[istarkov]

Too hard to remember all that pathes, defaults etc.
At the beginning I used a lot of handwritten HOCs over the React, now just few won the battle
In 99% of time I use withProps, withState, withPropsOnChange, withHandlers, defaultProps all others are rare,
before today I even had no knowledge that we have renameProps here ;-)

Now about types: we overuse dynamic nature of js language, instead of just numbers, strings, structs etc, we use Maybe types, (string | undefined) everywhere, why? A lot of years I wrote on non js languages and had no issues with living without undefined ;-)
Going deeper we even have that for every object we have not only undefined but additional state for every prop - does prop exists in an object (it's some kind of undefined but not the same).

I see your pain with construct above, I have the same, but solving it using magic paths with strings
we don't make our code simpler or better, every time I see the string path above I need to find a documentation how this library will resolve it, will it throw, will it provide some kind of defaults or what?

[josepot]

Since this is a "library-like" utility function, maybe it would be worth it to mutate in the reducer. It's ugly, sure, but it performs better:

      ...propNames.reduce(
        (flattenedProps, propName) =>
          Object.assign(flattenedProps, props[propName]),
        {}
      )

If a tree falls in the woods, does it make a sound?

If a pure function mutates some local data in order to produce an immutable return value, is that ok?

— Rich Hickey, Clojure

[istarkov]

every pure function is impure at assembler level - istarkov :-)

[gregberge]

I suggest removing flattenProp in favor of flattenProps, it would be very nice to have an unified interface for higher-order components taking path(s).

pickProps(paths)
omitProps(paths)
flattenProps(paths)
onlyUpdateForProps(paths)

What do you think?

[wuct]

I don't like the idea of the lodash-style path for same reasons @istarkov mentioned: it is not type friendly and needs more docs to introduce our implementation, but I know other people may like it.

The good part of FP is that you can compose your function easily. IMO letting the consumers of recompose decide how to get values from props by providing mapProps and withProps is all we need to do. Consumers can use these functions with other libraries they like.

[wuct]

There are many ways in JS to access deeply nested value. The lodash style is only one of them, and, IMO, which is not the best one :)

[alex-wilmer]

@wuct okay. well I'll close this then. just give us a version with deprecation warnings before you remove everything :)

[wuct]

@alex-wilmer sure!

[gregberge]

I introduced flattenProps and deprecated flattenProp in recompact.

@wuct I got your point, for me FP is the best way, we can compose everything with mapProps. But I think we should also have a consistent API and provide some "out of the box" higher-order components.