@wordpress/data: Introduce new custom `useDispatch` react hook #15896

nerrad · 2019-05-29T16:19:20Z

Description

This pull contains the work exposing a new useDispatch hook on the @wordpress/data package. As per discussions in #15473 and in #core-js slack meeting, the primary api exposed for the new hook is:

useDispatch( storeName: ?string ): Object|Function

storeName is optional and if provided then the action creators registered on that store are returned. Otherwise if no storeName is provided, then the registry.dispatch function is returned.

Also in this pull I've experimented with a useDispatchWithMap hook to replace withDispatch internals with (and we could maybe not expose this initially, but just implement in withDispatch only). I was interested in seeing:

a. is it possible.
b. does it simplify logic
c. what impact does it have.

I have a working example in this pull and did some performance benchmarking with the following results:

5.8.0 Tag

Average time to load: 7834ms
Average time to DOM content load: 7073ms
Average time to type character: 86.05ms
Slowest time to type character: 246ms
Fastest time to type character: 60ms

Current Master (at time of writing this post)

Average time to load: 6161ms
Average time to DOM content load: 5698ms
Average time to type character: 79.06ms
Slowest time to type character: 164ms
Fastest time to type character: 57ms

This branch

Average time to load: 7730ms
Average time to DOM content load: 7199ms
Average time to type character: 100.29ms
Slowest time to type character: 209ms
Fastest time to type character: 68ms

With those results, I doubt we'll want to merge this with the useDispatchWithMap changes and instead just keep withDispatch as is. Still, I'm sure there's better ways of doing that hook...

Things to do:

How has this been tested?

Screenshots

Types of changes

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

nerrad · 2019-05-29T16:21:00Z

@youknowriad, @epiqueras, and @aduth I'd love to get your feedback on this before I go further (note not in it's final state with inline docs etc).

youknowriad · 2019-05-29T16:23:03Z

packages/data/src/components/use-dispatch/use-dispatch-with-map.js

+	this( registry.dispatch, registry )[ propName ]( ...args );
+}
+
+const useDispatchWithMap = ( dispatchMap ) => {


So this is just something we'd use in withDispatch and not expose right?

I have another idea I'm going to try here too for performance. I think I could return a mutated ref object (which might help reduce renders).

Ya the idea I tried didn't help so not going with it.

nerrad · 2019-05-29T16:58:07Z

packages/data/src/components/use-dispatch/use-dispatch.js

+
+const useDispatch = ( storeName ) => {
+	const registry = useRegistry();
+	return useMemo( () => {


I wonder if useMemo is even needed here? Probably not?

What's more expensive? Calling dispatch or the hook's overhead?

Can registry change without dispatch changing? If so, you could make line 12:

const dispatch = useRegistry().dispatch;

And just close over that instead.

yeah I think I'm going to drop the use of useMemo. Even if registry changes it'll always expose dispatch. So I'll probably do something like:

const useDispatch = ( storeName ) => { const { dispatch } = useRegistry(); return storeName === void 0 ? dispatch : dispatch( storeName ); }

So calling dispatch on every render is fine (cheaper than the hook's overhead)?

Essentially, registry.dispatch is just an object of stores mapped to action creators, so there should be little to no overhead.

see 81ee6ee

Can you benchmark both approaches? This will be used in so many places that slight performance differences can significantly affect the user.

Right now the hook isn't used anywhere, so to use the performance tools we have right now I'd have to implement it in a few components that are tested with the performance tools (or there may be some other benchmarking approach I'd need to take). If you have some ideas on how I could effectively benchmark both approaches I'm all ears.

Otherwise, I think once the hook starts getting implemented throughout the codebase we can take a look at doing some benchmarking?

Timing dispatch(...) vs. useMemo(...) would work.

epiqueras

Great stuff @nerrad 🚀

I added a few suggestions and a fix for the performance regression you found.

packages/data/src/components/use-dispatch/use-dispatch-with-map.js

epiqueras · 2019-05-29T17:29:32Z

packages/data/src/components/use-dispatch/use-dispatch-with-map.js

+					`Property ${ propName } returned from dispatchMap in useDispatchWithMap must be a function.`
+				);
+			}
+			return proxyDispatch.bind( dispatchMap, propName, registry );


You could also just declare an arrow function here to avoid the this complexity.

I just ended up returning the dispatcher variable instead (see 7d88dd1)

You still need to call mapDispatch again when a dispatcher is called, so the selected state is fresh.

so something like this instead?

return ( ...args ) => dispatchMap( registry.dispatch, registry )[ propName ]( ...args )

Exactly 👍

actually I had to implement useRef to grab the latest dispatchMap (see 5856f20) Some failing e2e tests revealed that the above example I tried was not sufficient.

See #15896 (comment)

packages/data/src/components/use-dispatch/use-dispatch-with-map.js

epiqueras · 2019-05-29T17:34:10Z

packages/data/src/components/use-dispatch/use-dispatch.js

+
+const useDispatch = ( storeName ) => {
+	const registry = useRegistry();
+	return useMemo( () => {


What's more expensive? Calling dispatch or the hook's overhead?

Can registry change without dispatch changing? If so, you could make line 12:

const dispatch = useRegistry().dispatch;

And just close over that instead.

nerrad · 2019-05-29T22:18:58Z

Performance metrics from the latest commit 7d88dd1:

Average time to load: 6410ms
Average time to DOM content load: 5893ms
Average time to type character: 92.51ms
Slowest time to type character: 192ms
Fastest time to type character: 68ms

I retested master because it seemed the performance tests were running slower on my machine and there was a bit of a difference from the original post:

Average time to load: 6831ms
Average time to DOM content load: 6220ms
Average time to type character: 105.105ms
Slowest time to type character: 266ms
Fastest time to type character: 61ms

So it actually looks like things are running a tad bit faster now :) The variations in metrics are not that significant so I think we can roll with this approach? Any agreement here? If so, I'll get this pull finished up with:

exposing useDispatch
replacing internal withDispatch logic with the new useDispatchWithMap hook (but not exposing that hook to the api).
...tests, docs etc.

nerrad · 2019-05-30T02:11:46Z

packages/data/src/components/use-dispatch/use-dispatch-with-map.js

@@ -26,7 +32,9 @@ const useDispatchWithMap = ( dispatchMap ) => {
 						`Property ${ propName } returned from dispatchMap in useDispatchWithMap must be a function.`
 					);
 				}
-				return dispatcher;
+				return ( ...args ) => {
+					return currentDispatchMap.current( registry.dispatch, registry )[ propName ]( ...args );


I wonder if there still is a flaw with this though. dispatchMap might be the same function, but return a different collection of props between rerenders. In which case, this will only invoke the props functions returned on the intiial render (unless the function changes).

Eg. a map that looks something like this:

const mapDispatch = ( dispatch ) => { if ( ! props.foo ) { return {}; } return { onClick: dispatch.('someStore').addFoo( props.foo ) }; };

dispatchMap is a dependency there so it will re-run and return a new object.

What we can do is accept a deps argument and memoize dispatchMap at the top just like in useSelect.

That would give it all the properties we want:

The returned object with action dispatchers only changes when dispatchMap changes.

dispatchMap only changes if it has deps and any of them change.

So for your example you would call it like this: useDispatchWithMap( mapDispatch, [ props.foo ] ).

You also don't need any refs this way.

Keep in mind:

We're not exposing useDispatchWithMap for the general api, it's only intended for withDispatch internals.

withDispatch can't pass any dependencies for the same reason it wouldn't work in the withSelect implementation of useSelect because props shape changes.

My example is admittedly not clear because it would actually be something like this fed to withDispatch (and passed through here via withDispatch).

const SomeWrappedComponent = withDispatch( ( dispatch, { foo } ) => { if ( ! foo ) { return {}; } return { onClick: dispatch.('someStore').addFoo( foo ) }; } )( SomeComponent );

For reference, here's the code that was failing in e2e tests which the useRef implementation fixed:

gutenberg/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js

Lines 101 to 115 in 13e5851

export default compose( [

withSelect( ( select ) => ( {

isModalActive: select( 'core/edit-post' ).isModalActive( MODAL_NAME ),

} ) ),

withDispatch( ( dispatch, { isModalActive } ) => {

const {

openModal,

closeModal,

} = dispatch( 'core/edit-post' );

return {

toggleModal: () => isModalActive ? closeModal() : openModal( MODAL_NAME ),

};

} ),

] )( KeyboardShortcutHelpModal );

In that case, this should happen:

Prop changes make the wrapped component rerender.

mapDispatch is redefined:

gutenberg/packages/data/src/components/with-dispatch/with-dispatch-new.js

Line 14 in de2d19d

const mapDispatch = ( dispatch, registry ) => mapDispatchToProps(

The hook's useMemo clears and returns a new object:

gutenberg/packages/data/src/components/use-dispatch/use-dispatch-with-map.js

Line 39 in de2d19d

}, [ dispatchMap, registry ] );

ya that's what I expected too. But both the e2e test and manual testing were failing for that behaviour (when using the keyboard shortcut for that modal when it was open, isModalActive was always false - as if it was closed). It might be that this is exposing an issue somewhere else? (btw there were a couple other components failing too, this is just the easiest to point to).

sidenote: thanks for being responsive in this pull... I'm off for the night though 👋

Yeah I found the reason why.

gutenberg/packages/components/src/keyboard-shortcuts/index.js

Line 25 in 13e5851

componentDidMount() {

That component binds the callbacks on mount and never updates them.

I guess we'll have to keep the ref then. But I think we should reference the ref instead of the variable everywhere for consistency. We should also use useLayoutEffect instead of useEffect to avoid action dispatchers called between renders and effects having an outdated callback.

Yeah it's getting pretty late here too. Good night!

nerrad · 2019-05-30T02:22:01Z

The latest useDispatchWithMap iteration pushed doesn't affect the performance significantly and its pretty similar to master. The performance tests are fluctuating for me (sometimes a difference of 10-15ms for average typing speed between tests) but the variance between master and this branch seems to be about the same. Would be good to have someone else run the performance tests too.

youknowriad · 2019-05-31T13:50:28Z

packages/data/src/components/use-dispatch/use-dispatch-with-map.js

+					.current( registry.dispatch, registry )[ propName ]( ...args );
+			}
+		);
+	}, [ dispatchMap, registry ] );


So this means this is executed each time dispatchMap changes and from the implementation of withDispatch, the function passed is an inline one which means this will be regenerated on each call. This I believe defeats the purpose here.

Also, I don't mind if withDispatch is not refactored to use a hook right now but I'm opposed to it either.

Yeah, but it's fine because it's wrapped in pure so that will only happen when props change.

I'd still like someone else to verify my findings with performance tests but from the ones I've run the impact is pretty much nil either way (though I have seen more small improvements in successive runs than regressions).

The idea of the proxy dispatches is to avoid running the mapping function each time the props change to avoid rerendering sub-components (have always the same reference to the functions returned by withDispatch across rerenders). This was one of the main differences with connect.

How is this addressed if the the action creators are regenerated each time props change.

I'd still like someone else to verify my findings with performance tests but from the ones I've run the impact is pretty much nil either way

I think ideally, you should disable the "async" mode and do the performance tests to compare exactly the impact. The async mode hides some of the impact by only focusing on the selected block which is going to be rerendered anyway.

You are right. If we assume that the keys of the returned object won't change. We can do what I described here: #15896 (comment) and pass in [] as the deps in withDispatch. That would achieve what we want.

If we assume that the keys of the returned object won't change. We can do what I described here: #15896 (comment) and pass in [] as the deps in withDispatch. That would achieve what we want.

As I mentioned in my reply to your previous comment, the problem here is what would we use as the value for deps from withDispatch? We can't use Object.values( props ) because of the same issue encountered in useSelect because the props object shape is not consistent.

I think ideally, you should disable the "async" mode and do the performance tests to compare exactly the impact. The async mode hides some of the impact by only focusing on the selected block which is going to be rerendered anyway.

Do you have some pointers on how I can do that for the performance tests? If you don't have anything in mind I can look into that (I'm assuming just temporarily modify the AsyncMode provider? for the build the tests are run on). Although I'm not sure that will give accurate results either because then we won't know if the performance hit is from the selects in the tests or the dispatch right?

With the uncertainty around impact here, I'm inclined to just do useDispatch in this pull and leave withDispatch as is.

Do you have some pointers on how I can do that for the performance tests? If you don't have anything in mind I can look into that (I'm assuming just temporarily modify the AsyncMode provider?

Pass false to this prop https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/block-list/index.js#L212

Although I'm not sure that will give accurate results either because then we won't know if the performance hit is from the selects in the tests or the dispatch right?

yes, but I was thinking that if you compare this branch with master, the only changes are in withDispatch

As I mentioned in my reply to your previous comment, the problem here is what would we use as the value for deps from withDispatch? We can't use Object.values( props ) because of the same issue encountered in useSelect because the props object shape is not consistent.

An empty array, it never needs to re-run because the callbacks reference the mutable ref.
In this case deps would be: depencies that make Object.keys(mapDispatch(...)) return a different list.

yes, but I was thinking that if you compare this branch with master, the only changes are in withDispatch

Ahh right.

An empty array, it never needs to re-run because the callbacks reference the mutable ref. In this case deps would be: depencies that make Object.keys(mapDispatch(...)) return a different list.

I apologize for not quite getting it yet. Do you mean withDispatch would call useDispatchWithMap like this:

const dispatchProps = useDispatchWithMap( dispatchMap, [] );

And then the signature for the hook would be something like this:

const useDispatchWithMap( dispatchMap, deps ) { dispatchMap = useCallback( dispatchMap, deps ); /** mostly existing logic **/ }

My bad, I should have been more clear.

Yeah exactly, but the ref should always be updated from the passed in parameter (not the memoized variable).

Now that I think about it more. Replacing mapDispatch with ...deps in the useMemo deps would be cleaner and have the same effect.

ensures that any state is fresh when prop functions are called.

This more closely mirrors the original behaviour of withDispatch.

nerrad · 2019-06-01T13:13:39Z

New `withDispatch` (using `useDispatchWithMap` hook) vs old `withDispatch` performance tests.

Here's some performance metrics on the latest push (6946a6a). For "no Async" tests, I used the method mentioned by @youknowriad here. For all tests in each branch I did three runs and then averaged the values across those runs.

Branch	avg load	avg dom load	avg type	slowest type	fastest type
useDispatch - with async	7246ms	6638ms	101ms	240ms	69ms
master - with async	6790ms	6208ms	98.92ms	227ms	56ms
useDispatch - no async	9190ms	7878ms	280.841ms	509ms	223ms
master - no async	9035ms	7590ms	293ms	631ms	219ms

Conclusions:

master is faster than this branch for the "with async" tests for the average but oddly with no async (which more closely tests the changes in withDispatch), this branch is faster! On the whole this suggests that there are some marginal gains with the new withDispatch and some minor negative impact on overall performance.

`useDispatch` with `useMemo` and without `useMemo` performance tests.

The purpose of these tests (as suggested here was to see whether memoizing the returned action creators (via useMemo) was slower or faster than the currently implemented logic. For the test, I created a mock component implementing useDispatch with a mock registry and used react-test-renderer to mount and unmount the component 1000 times. I tested invoking useDispatch both with a store name and without the store name argument. The results are below:

type	Avg time to render with store name	Avg time to render (no store name )
no `useMemo`	0.048ms	0.03ms
with `useMemo`	0.049ms	0.032ms

Conclusions

It appears that the difference between the two is very marginal but there is an overhead cost (however minor) to implementing useMemo so it appears we're good to leave the current logic as is.

Overall, from a performance perspective, I think there's nothing indicating that we need to backtrack on some of these changes. I'll continue on with covering all the changes in unit tests.

youknowriad · 2019-06-01T13:37:36Z

packages/data/src/components/with-dispatch/index.js

-			render() {
-				return <WrappedComponent { ...this.props.ownProps } { ...this.proxyProps } />;
-			}
+	( WrappedComponent ) => pure(


I'm pretty sure this pure call is not really necessary (and I suspect the small decline of performance that you noticed is coming from it)

oh good point, I'll try without it.

Removed in 6500117. The new 3 run average for the new withDispatch after pure being removed is:

Average time to load: 6590ms
Average time to DOM content load: 6041ms
Average time to type character: 99ms
Slowest time to type character: 234ms
Fastest time to type character: 71ms

So ya, it does look like there was some unnecessary overhead added by using pure here.

youknowriad · 2019-06-03T08:56:26Z

packages/data/src/components/use-dispatch/use-dispatch.js

+ */
+const useDispatch = ( storeName ) => {
+	const { dispatch } = useRegistry();
+	return storeName === void 0 ? dispatch : dispatch( storeName );


First time I see === void 0. I'd have done !! storeName personally. Curious to learn more here.

ya !! storeName could have been used here. Using void 0 is just an old habit I sometimes slip into when I want to check against the undefined primitive value. I don't know if it's still the case with modern browsers, but it was possible for the undefined global to be overwritten so it was just a protection.

I don't know if it's still the case with modern browsers, but it was possible for the undefined global to be overwritten so it was just a protection.

It's not been possible to override in any browser which conforms to ES5 and newer (IE9+).

https://www.ecma-international.org/ecma-262/5.1/#sec-15.1.1.3

A future maintainer most certainly will struggle to under what this code is doing. Not a big issue, but in general we should feel fine to compare against undefined.

Babel might even take care of it too.

I feel really old all of a sudden 😉

Babel might even take care of it too.

I was curious about this, and it doesn't seem like Babel does, but Uglify will convert it, yes.

Interesting, maybe for the older targets?

Likely because void 0 is 3 fewer characters than undefined 😄

Likely because void 0 is 3 fewer characters than undefined

Ha! That's my new reason.

youknowriad

This seems ready to land. Great work.

aduth · 2019-06-04T19:01:58Z

packages/data/src/components/use-dispatch/use-dispatch-with-map.js

+ *                                the first argument and the `registry` object
+ *                                as the second argument.  Should return an
+ *                                object mapping props to functions.
+ * @param {Array}    deps         An array of dependencies for the hook.


What purpose does deps serve if this is an internal hook, and the only place we use it, we always pass an empty array?

That's a good point. Right now with this being an internal only implementation it's obviously an unneeded variable. There may be potential for this hook being exposed at some point as a public api (if use-case demonstrates need) in which case there is utility for the deps arg.

aduth · 2019-06-04T19:05:25Z

packages/data/src/components/with-dispatch/index.js

 * } )( Button );
 *
 * // Rendered in the application:
 * //
 * //  <SaleButton>Start Sale!</SaleButton>
 * ```
- * _Note:_ It is important that the `mapDispatchToProps` function always returns an object with the same keys. For example, it should not contain conditions under which a different value would be returned.


Do you not foresee it being possible in the future for optimizations which leverage the fact we'd established an expectation that the prop names don't change? The previous implementation benefitted by the fact we could compute the proxy functions once for the entire lifecycle of a component. I'm not sure if or how it would make sense to reflect that in the new implementation, but changing the documentation here may risk eliminating this as an option to explore down the line.

Oh, I removed this because I thought it was documentation I had added in iterating on this pull! I agree we should keep this in place. I'll add back in a separate pull (might not be till later)

youknowriad reviewed May 29, 2019

View reviewed changes

nerrad commented May 29, 2019

View reviewed changes

epiqueras requested changes May 29, 2019

View reviewed changes

nerrad commented May 30, 2019

View reviewed changes

youknowriad reviewed May 31, 2019

View reviewed changes

nerrad added 14 commits May 31, 2019 20:27

initial commit with two new dispatch hooks.

d54e692

replace non-function property values with a function

0db22f7

try alternate approach

2c15377

try with useMemo

2f1e32d

conditionally return either dispatch or specific store actions

6f3ee01

don’t use proxy and move initial dispatchMap call into useMemo callback

9c9df2f

remove unnecessary use of useMemo

9dcce6f

make sure latest dispatchMap is invoked

690830c

ensures that any state is fresh when prop functions are called.

shorten syntax

5ee59b6

replace old withDispatch with new

89a7fb4

add documentation and export useDispatch

70845bb

add documentation and implement useLayoutEffect

33293b2

update to README

50e8a70

implement deps on useDispatchWithMap

6946a6a

This more closely mirrors the original behaviour of withDispatch.

nerrad force-pushed the FET/useDispatch-hook branch from 70f2bea to 6946a6a Compare June 1, 2019 00:31

youknowriad reviewed Jun 1, 2019

View reviewed changes

nerrad added 4 commits June 1, 2019 15:32

remove unnecessary pure call and improve docs

6500117

update with-dispatch tests

4b19d7c

fix jsdoc for optional param

5137db6

add tests for useDispatch hook

16b2b04

nerrad marked this pull request as ready for review June 1, 2019 20:50

nerrad requested a review from aduth as a code owner June 1, 2019 20:50

nerrad added 4 commits June 1, 2019 16:52

add changelog entry

0426938

fix spacing in documentation

540860b

more spacing fixes

21189e7

more documentation spacing fixes

326fba1

youknowriad reviewed Jun 3, 2019

View reviewed changes

youknowriad approved these changes Jun 3, 2019

View reviewed changes

nerrad merged commit 9780614 into master Jun 3, 2019

youknowriad deleted the FET/useDispatch-hook branch June 3, 2019 11:28

aduth mentioned this pull request Jun 4, 2019

Data: Implement useSelect hooks corollary to withSelect #15473

Closed

aduth reviewed Jun 4, 2019

View reviewed changes

nerrad added a commit that referenced this pull request Jun 4, 2019

add important clarification doc removed in #15896

c1a4743

nerrad mentioned this pull request Jun 4, 2019

Add back important clarification doc for withDispatch removed in #15896 #15987

Merged

gziolo added [Package] Data /packages/data [Type] Task Issues or PRs that have been broken down into an individual action to take labels Jun 5, 2019

gziolo added this to the 5.9 (Gutenberg) milestone Jun 5, 2019

nerrad added a commit that referenced this pull request Jun 5, 2019

add important clarification doc removed in #15896 (#15987)

705497b

	export default compose( [
	withSelect( ( select ) => ( {
	isModalActive: select( 'core/edit-post' ).isModalActive( MODAL_NAME ),
	} ) ),
	withDispatch( ( dispatch, { isModalActive } ) => {
	const {
	openModal,
	closeModal,
	} = dispatch( 'core/edit-post' );

	return {
	toggleModal: () => isModalActive ? closeModal() : openModal( MODAL_NAME ),
	};
	} ),
	] )( KeyboardShortcutHelpModal );

@wordpress/data: Introduce new custom useDispatch react hook #15896

@wordpress/data: Introduce new custom useDispatch react hook #15896

Conversation

nerrad commented May 29, 2019 • edited

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

nerrad commented May 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epiqueras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerrad May 30, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerrad commented May 29, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerrad May 30, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerrad May 30, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerrad commented May 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerrad commented Jun 1, 2019

New withDispatch (using useDispatchWithMap hook) vs old withDispatch performance tests.

Conclusions:

useDispatch with useMemo and without useMemo performance tests.

Conclusions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerrad Jun 4, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

@wordpress/data: Introduce new custom `useDispatch` react hook #15896

@wordpress/data: Introduce new custom `useDispatch` react hook #15896

nerrad commented May 29, 2019 •

edited

nerrad May 30, 2019 •

edited

nerrad commented May 29, 2019 •

edited

nerrad May 30, 2019 •

edited

nerrad May 30, 2019 •

edited

New `withDispatch` (using `useDispatchWithMap` hook) vs old `withDispatch` performance tests.

`useDispatch` with `useMemo` and without `useMemo` performance tests.

nerrad Jun 4, 2019 •

edited