Components: Add withState higher-order component

[aduth]

Supersedes #3435

This pull request seeks to add a new withState higher-order component to enable ES5 block implementers to introduce state to their components without reaching for the Component class.

View documentation

Included is an example porting the HTML block component to a stateless function component using withState.

Testing instructions:

Verify that there are no regressions in the behavior of toggling between Preview and HTML in the HTML block.

[gziolo]

Should we start using React.Fragment to avoid using keys? Do we want to update Babel first? :)

[aduth]

Should we start using React.Fragment to avoid using keys? Do we want to update Babel first? :)

Yeah, I'd probably want to avoid the direct reference to React.Fragment, but if we had <> syntax available, I'd be open to using it. Maybe in a subsequent pull request where we upgrade Babel?

[gziolo]

Sure, I can spin up PR with Babel upgrade on Monday :)

[gziolo]

Just noting that React docs discourage using setState( { count: count + 1 } ) this way when it depends on props:

React may batch multiple setState() calls into a single update for performance.

Because this.props and this.state may be updated asynchronously, you should not rely on their values for calculating the next state.

They suggest using the following:

setState( count => ( { count: count + 1 } )

See https://reactjs.org/docs/state-and-lifecycle.html#state-updates-may-be-asynchronous. I think we should follow those guidelines in here, and in the unit test.

[aduth]

I think we should follow those guidelines in here, and in the unit test.

Updated in rebased 6fba36d.

[gziolo]

In other places, I omitted with prefix for the display name. We should either update all existing getWrapperDisplayName calls or put state here instead. Both work for me.

[aduth]

We should either update all existing getWrapperDisplayName calls or put state here instead.

Updated in rebased 6fba36d.

[gziolo]

I like this HOC a lot. This is exactly what we need to make it possible to use state with functional components or when using ES5 only. Great work 💯

I left some minor comments. They aren't blockers, but taking into account that we are updating docs we might want to be super strict and align with React docs :)

[lamosty]

This is great, thanks for doing it!

[youknowriad]

Do we really want to add an maintain this kind of API? Do we really want to optimize for the ES5 use-case?

• If we want a similar API, should we refer to recompose in the docs for example?
• If we want friendly ES5 components, should we refer to create-react-class instead (or expose it). If this PR is meant to address the ES5 issue, it only does so for the state, what if someone wants to use the lifecycle methods?

[gziolo]

I would advocate for full support for ES5 style as long as it is the only way to avoid build step.

If we want a similar API, should we refer to recompose in the docs for example?

We might advertise recompose library as a good way to deep diver into HOC world.

If we want friendly ES5 components, should we refer to create-react-class instead (or expose it). If this PR is meant to address the ES5 issue, it only does so for the state, what if someone wants to use the lifecycle methods?

create-react-class isn't fully compatible with ES6 class component. Personally, I would offer withLifecycleMethods HOC instead to make it possible to mirror the original React 16 behavior. Actually I will ask on Twitter what React core team thinks about such idea and if there are any potential drawbacks.

[aduth]

Do we really want to optimize for the ES5 use-case?

Personally I think we should seek to be as inclusive as possible to those who choose not to include a build step. Also, arguably one of the points of this change was that it's an obvious pattern for introducing state even for those who choose to use newer language syntax.

should we refer to recompose in the docs for example?

Recompose includes a fair bit more functionality that may be overwhelming for those who are simply trying to maintain some local component state. It's hard to draw a line between what we do and don't want to support and maintain ourselves. I think something like this covers a pretty good baseline option without needing to reach for more powerful solutions.

If we want friendly ES5 components, should we refer to create-react-class instead (or expose it).

As mentioned in #3435, I don't think we should encourage create-react-class. From what I can tell, it exists largely for backwards-compatibility with React.createClass, including some now-defunct "surprises" like auto-binding.

it only does so for the state, what if someone wants to use the lifecycle methods?

We could introduce a HoC for lifecycle, but instead I'd be curious to identify the use-cases that would call for lifecycle in block implementations and see if we can address those with more targeted utilities.

[gziolo]

We could introduce a HoC for lifecycle, but instead I'd be curious to identify the use-cases that would call for lifecycle in block implementations and see if we can address those with more targeted utilities.

It might make sense to have withEventHandlers which would bind/unbind event handlers. We would need to go through the existing code and identify what is really necessary. It wouldn't be a bad idea to convert a few existing blocks to use such HOCs instead of Component.

[mtias]

Thanks for working on this. I do like this API and would like to see how we could use it for our own components as well, as it seems inherently more flexible in not tying to a library but tying to WP patterns instead.

[gziolo]

I asked on Twitter the following:

If you could shape React’s public API again, would you use expose class syntax for the component or rather use functional approach with higher order functions that add state support or lifecycle methods through composition?

Replay from Dan Abramov:

See my reply to a similar question in https://dev.to/dan_abramov/comment/1n29