Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

Using event.stopPropagation() in withStateHandlers does not work. #602

Open
philipnilsson opened this issue Feb 2, 2018 · 4 comments
Open

Comments

@philipnilsson
Copy link

philipnilsson commented Feb 2, 2018

const Component = withStateHandlers(
  () => ({}),
  {
     onUpdate: () => e => {
       e.stopPropagation();
     }
  }
);

Expected

This code will stop event-propagation.

Actual

It doesn't

Cause

Handlers are run asynchronously inside a setState closure, and so stopPropagation is not called on time.

Solution

Change this line to run handlers eagerly, and not inside the closure.

@istarkov
Copy link
Contributor

istarkov commented Feb 2, 2018

This is expected. Line change will not help as it will break atomic setState updates. Use intermediate withHandlers to stopPropagation if needed or stop it directly in event handler ie onClick={e => {e.stopPropagation(); handleBlabla(e);}}
withStateHandlers is a flowtyped replacement of withState and not intended to have same abilities as withHandlers.
I'll close this as it's not a bug or anything bad.

@istarkov istarkov closed this as completed Feb 2, 2018
@philipnilsson
Copy link
Author

Thanks @istarkov. It's very surprising, though, lost some time to this.

In theory it would be possible to swap the order of the curried arguments, right?

event => (state, props) => { ... }. Then one could at least stop the propagation in the outer closure?

@wuct
Copy link
Contributor

wuct commented Feb 11, 2018

@philipnilsson thanks for your advice. I think swapping the order of the arguments do help. Besides, there is no need to call event.persist() inside withStateHandlers because now we can access the event synchronously. I've created a code sandbox for this idea.

@istarkov how do you think?

@ElvenMonky
Copy link

ElvenMonky commented Aug 28, 2018

How come withStateHandlers was introduced with incorrect call order for stateUpdaters in the first place!

It seems obvious that setState should be called inside event handler, and not other way round.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants