Prune list of helpers

[acdlite]

As we start thinking about a 1.0 release, I want to drop any helpers that haven't worked out or are out of scope. Let's use this issue to keep track of which ones are on the chopping block:

• withAttachedProps — will be replaced by withHandlers (I'll have PR ready for this soon added in 0.17)
• mapPropsOnChange — not convinced this is actually useful. Can be replaced by normal mapProps + function memoization, via lodash or otherwise. Keeping this and renaming to withPropsOnChange() mapPropsOnChange -> withPropsOnChange #134
• doOnReceiveProps
• lifecycle

Update: there's some disagreement over whether the lifecycle-based helpers belong in the library or not. My attitude is that they're currently half-baked, so let's remove them for now and we can add them in a minor release once we've figured out a better story for them — or not, if we decide they're outside the scope of the library.

[istarkov]

Without lifecycle it will be impossible to cancel async tasks.

[istarkov]

BTW I'm not using lifecycle directly, and use my cancelAsync HOC based on lifecycle, which can be easily be rewritten as simple component.

[nfcampos]

doOnReceiveProps is pretty useful, especially as a building block for other things, it's basically createSink but you can also render something

[acdlite]

My thinking is that both lifecycle and doOnReceiveProps are too leaky. @nfcampos Can you give me an example of how you've used doOnReceiveProps?

[RafalFilipek]

+1 for withAttachedProps & mapPropsOnChange
no opinion - doOnReceiveProps
lifecycle - should we use classes for that?

[acdlite]

@RafalFilipek Yes, or something like rx-recompose's observeProps

[nfcampos]

@acdlite actually you're right, it's not that useful, at least for me, I thought I remembered two times I had used it but going back to find them I now realise that

• in one it was just a pretty stupid use so nevermind ;)
• in the other I thought about using it but ended up not using it because I also needed access to the old props inside the callback (to compare) (this 2nd use was this https://gist.github.com/nfcampos/512076429479c7c9b7243be0b33ec924 a HOC like withProps but for props that are computed asynchronously, if you think something like that belongs in recompose I'd be more than happy to contribute)

[istarkov]

@acdlite wrote mapPropsOnChange — not convinced this is actually useful. Can be replaced by normal mapProps + function memoization, via lodash or otherwise.

I think It can't be replaced normal.

Because:

• default lodash memoize just uses first argument as a key for memoize cache, creating universal resolver to create cache key for multiple arguments is not a big problem but it will be not fast operation (find on a list of used arguments). As I use mapPropsOnChange on thousands of created - removed element, this find operation will be a performance problem.
• to use memoize like in reselect https://github.com/reactjs/reselect/blob/master/src/index.js#L5-L20 which memoizes only last call, we will need additional helper to create memoized function for every element created from current Component. Like lifecycle.
• even with good memoize we need to define function body outside of mapProps, but for me it is the biggest advantage of recompose (and std map filter reduce) that I can place anonymous function in that place where it used.
• it will be impossible to use universally some current mapPropsOnChange behavior, for example using in computation props which is not in depdendentPropKeys list.

IMHO mapPropsOnChange is very useful HOC for components creation, and I use it very often (but not current version, I use this #119)

For example for my Chart component (not published yet) I use

• mapPropsOnChange 33 times
• mapProps 10 times

as there are a lot of computationally intensive tasks for chart data, which can be split in a lot of small operations.

Or here is live example https://github.com/istarkov/google-map-clustering-example/blob/master/src/GMap.js

• mapPropsOnChange 3 times
• mapProps 0 times

[npbee]

Regarding withAttachedProps, I ended up using that a bit for attaching immutable props to components other than just event handlers. So I guess like a one-time mapProps function. As a silly example:

// This is a higher-order component that expects a `fullName` prop
const someHoc = ComposedComponent => props => {
    // Do something with props.fullName
};


// Person.js
const Person = props => <div>{props.fullName}</div>;

export default compose(
   withAttachedProps(getProps => {
      const { firstName, lastName } = getProps();

      return { fullName: `${firstName} ${lastName}` };
   }),
   someHoc
)(Person)


// Later...
<Person firstName='Tom' lastName='Cruise' />

So I needed to compute props in order to provide new props to a child component, but I knew that I only needed to compute those props once.

Would there be a better way to do this with one of the other helpers? Or perhaps is this just a bad idea all around?

[acdlite]

@npbee I believe that's what the original idea behind mapPropsOnChange was. This is a real use case; need to come up with the best API for it.

[acdlite]

@nfcampos I've thought about adding something like your withPromisedProps but it really only works on the initial render. In my app, I've been using observeProps + flatMap for this use case, because observables can handle multiple values over time.

[nfcampos]

@acdlite I think i'll end up doing something like that

[dlebedynskyi]

I liked doOnReceiveProps was pretty helpful. only one thing that could make it better is to pass oldProps as second argument.
it is greatly simplifies some code
from

componentWillMount(){
this.init(this.props, {});
}
componentWillReceiveProps(nextProps) {
this.init(nextProps, this.props);
}

to

compose(doOnReceiveProps(init))

basically removing need to create a class entirely. It is pretty handy with redux and multiple parts of store required. They not always as easy to describe with one promise since status can change by different actions such as sockets message

[ilyagelman]

Sorry to discover about dropping doOnReceiveProps – I used it in a few places when I needed some components to fetch missing pieces of data both on mount and update. Using this saves LOCs.

In fact I used lifecycle a few times as well.

[acdlite]

I'm not opposed to bringing back doOnReceiveProps() if someone details a use case, with specific examples, and suggests a better API. A PR with docs would be the best way to do this.

[istarkov]

One of most common use case for me for doOnReceiveProps is to have internal state with initial value equal or depending on some external prop, and for performance and other reasons I need to change this initial value every time external prop has changed.

As a simple example, I save on the server every component value change.
Let's take a simple slider component, it has value and onChange props, and on every drag value is changed.
This will generate a lot of onChange events, so the best is to save value of this component on onMouseUp event. (onBlur for inputs, etc)
So for a lot of such controls, I use HOCs with internal value state, which is updated as usual on onChange event and if external value has changed. (External value can be changed not only in onMouseUp event but as a result of some external operations)

The other use cases are similar, except where I need to call some external action.

So for most of my use-cases doOnReceiveProps is like withState where initialState callback is called not only on initial phase but also at every componentWillReceiveProps call with (props, nextProps, state) signature.
I played with this idea, but got that it's too complicated to read such code.

[istarkov]

PS: Looks like a lot of my current situations could be solved with rx-recompose, so I'll try to start using it.

[dlebedynskyi]

@istarkov rx-recompose can help with what you have. we are not using rx yet so standard helper was great for us

[belkosyak]

@acdlite please take a look at #159 . We often use doOnReceiveProps to fetch data, with is missing to show component. Also very useful to show some loading spinner while needed data is loading.