React: Custom field in event not cleaned up

Created on 29 Jul 2017  路  8Comments  路  Source: facebook/react

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
The custom field in event does not clean up. The data appear in other event fired.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/84v837e9/).

https://jsfiddle.net/84v837e9/175/

Current behaviour:
Reload -> Click yellow area (alert Hello!) -> Click green area (alert Hello!)
Reload -> Click green area (alert undefined)

What is the expected behavior?

Expect clicking yellow area always alert Hello! and clicking green area always alert undefined

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

I got this problem in v15.6.1, not sure does it affect earlier version.

Most helpful comment

I wrote a post to explain the conclusion of this issue.
https://medium.com/@sundaykuloksun/why-react-discourage-event-delegation-2b5fe3f52bea
Hope it can help others who have similar question.

All 8 comments

I don鈥檛 think it was ever meant to be supported to mutate the event object.
We should probably freeze it in development.

Hmm, we can鈥檛 freeze it because we pool it. Maybe we can preventExtensions() then.

@gaearon Thanks for your reply! But I have use case of custom event field on event delegation.

I cannot use the traditional event delegation pattern because of the abstraction of React

  • We use React component props instead of DOM data attribute to store data
  • React event handler got a DOM object instead of React component in target field of event parameter

I cannot know which component is clicked, without appending custom data to event object.

Do I mis-understand anything about React? Is there better solution than custom field to handle the event delegation patter?

I鈥檓 having trouble understanding what you鈥檙e trying to do. Can you provide a small example representative of your real world use case?

In React, people usually don鈥檛 rely on bubbling to pass data. Instead your component should handle an event, and possibly notify parent components by a callback. You can pass any data to such callback (e.g. this.props.onItemClick(data)), and some of this data might be based on e event, while other parts might be based on component props or be hardcoded.

@gaearon Thanks for your quick reply!

I am trying to avoid passing function to the children components because

  1. passing function to grand-children is difficult to maintain

    • (any change of the component in the middle will break the chain)

  2. avoid unnecessary re-render on manual mistake on passing function to props

    • (like <Foo onClick={ this.onClick.bind(this) }>)

  3. decoupling the parent component and children component

    • Parent container don't need to care the props of children component

    • Children component don't depends on function from outside, it only need to put its own data in the event

Here is my example: https://jsfiddle.net/84v837e9/177/

  1. <Parent/> don't not need to pass onClick hander to <Children/> and <Children/> don't need to pass handler to its own <Children/>
  2. No props pass from <Parent/> to <Children/>, can completely avoid unnecessary re-render
  3. No dependency between parent component and children component

    • <Parent/> don't need to know how the children component handle onClick event

    • <Children/> don't need to run function from outside, it is cleaner as all method is defined in the class


I know there are other ways to solve the my problems.

But I feel like event delegation like this is also clean and easy to maintain.

It may be also a good pattern to implement if React can support it.

passing function to grand-children is difficult to maintain
(any change of the component in the middle will break the chain)

It is more effort to write but once written, I would argue it actually helps maintenance because every connection is explicit. You can鈥檛 move components around and accidentally lose the connection between them (which is what often happens in logic relying on delegation).

avoid unnecessary re-render on manual mistake on passing function to props
(like

Not sure why you鈥檇 need event delegation to solve this? Bind the function in constructor, or use the class property syntax that does it for you.

handleClick = (e) => {
  // this function is bound
}

decoupling the parent component and children component

They are already coupled by logic. I think you鈥檙e trying to avoid reflecting this coupling in code, but the coupling is still there鈥攊t鈥檚 just implicit. In React apps it is preferable to keep these relationships between components explicit, even at the cost of writing more code.

I understand where you鈥檙e coming from, but this pattern is intentionally discouraged in React. Please use one-way data flow and pass callbacks down. This is more work in the beginning, but it keeps the data flow very explicit, and helps find bugs later on.

I wrote a post to explain the conclusion of this issue.
https://medium.com/@sundaykuloksun/why-react-discourage-event-delegation-2b5fe3f52bea
Hope it can help others who have similar question.

I wrote a post to explain the conclusion of this issue.
https://medium.com/@sundaykuloksun/why-react-discourage-event-delegation-2b5fe3f52bea
Hope it can help others who have similar question.

@gaearon Does that mean if the table has 500 rows and each row can be clicked on to do something (or maybe it is a delete or edit button in each row), there would be 500 event listeners added, one for each row? One place I visited said: We'll see how to attach events directly to elements and React takes care of event delegation and optimization for you.

Was this page helpful?
0 / 5 - 0 ratings