Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In React 16, onMouseEnter is triggered an extra time when entering a new child #10906

Closed
Sherryer opened this issue Sep 28, 2017 · 14 comments · Fixed by #11164
Closed

In React 16, onMouseEnter is triggered an extra time when entering a new child #10906

Sherryer opened this issue Sep 28, 2017 · 14 comments · Fixed by #11164

Comments

@Sherryer
Copy link

版本(versions):
react@16.0.0
react-dom@16.0.0

描述(description):
给父级别绑定 onMouseEnter 事件。在首次渲染后,鼠标移入新渲染进去的子元素会触发父元素的 onMouseEnter 事件。

示例(demo):

import React, {Component} from 'react';

class Demo extends Component {
    constructor(prop) {
        super(prop);
        this.enter = this.enter.bind(this)
        this.state = {
            flag: false
        }
    }
    enter(event) {
        console.log(event.nativeEvent);
        console.log("enter")
    }
    click() {
        this.setState({flag:!this.state.flag})
    }
    render() {
        let old = {
            height: "100px",
            width: "100px",
            border: "1px solid #6dbbff"
        };
        let aa = {
            height: "100px",
            width: "100px",
            border: "1px solid red"
        };
        let content = {
            border: "1px solid black",
            display: "flex",
            padding:"10px"
        }
        return(
            <div>
                <div style={content} onMouseEnter={this.enter}>
                    {this.state.a}
                    <div style={old}>old</div>
                    <div style={aa}>old</div>
                    {this.state.flag && <div style={aa}>new</div>}
                </div>
                <button onClick={this.click.bind(this)}>点我(click me)</button>
            </div>
        )
    }
}
@cyan33
Copy link
Contributor

cyan33 commented Sep 28, 2017

@Sherryer Do you mean you want the click button to be free from the onMouseEnter event handler?

@gaearon gaearon changed the title v16.0.0 onMouseEnter will be always triggered when enter the childNode which is created fresh In React 16, onMouseEnter is triggered an extra time when entering a new child Oct 3, 2017
@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2017

I can confirm I see the difference between 15 and 16 behavior. Maybe a bug.

@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2017

(On the side note, how awesome is it that we speak different languages, and yet can understand each other with code? 😄 )

@gaearon gaearon mentioned this issue Oct 3, 2017
19 tasks
@cyan33
Copy link
Contributor

cyan33 commented Oct 3, 2017

@gaearon Hey Dan, I created a fiddle about this issue.

https://jsfiddle.net/cyan3/k28vxab1/

If you click the button, and hover on the right most div('new'), it will trigger the onmouseenter event twice. It seems to me a bug.

If you need any help, I'm much willing to do something.

@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2017

I don’t really have any idea about where it’s coming from. So you can start looking into how the code path for that event is different (maybe in EnterLeavePlugin) between 15 and 16.

@jquense
Copy link
Contributor

jquense commented Oct 3, 2017

Another possible place is here: https://github.com/facebook/react/blob/master/src/renderers/shared/shared/ReactTreeTraversal.js#L112

There are a few enter/leave issues piling up maybe it's worth getting the switch to native events in there: #10269

@aweary
Copy link
Contributor

aweary commented Oct 3, 2017

To clarify, the behavior isn't that there are two enter events being dispatched on the same element; it's that the newly added element is also dispatching an enter event that is being captured by the parent's onMouseEnter handler. If you mouse over very slowly you'll see the first event fires when your mouse enters the parent (black border) and the second fires when your mouse enters the new child (red border).

@aweary
Copy link
Contributor

aweary commented Oct 3, 2017

Another possible place is here: https://github.com/facebook/react/blob/master/src/renderers/shared/shared/ReactTreeTraversal.js#L112

@jquense this looks like part of the root issue. I'm seeing that for the child divs that aren't triggering enter events, to and from inside traverseEnterLeave are equal. With the newly added div, to and from are not equal, so it ends up populating the pathTo and pathFrom arrays and the event gets processed.

@Sherryer Sherryer closed this as completed Oct 9, 2017
@gaearon
Copy link
Collaborator

gaearon commented Oct 9, 2017

It’s a valid bug, please don’t close it.

@gaearon
Copy link
Collaborator

gaearon commented Oct 9, 2017

The issue is with these checks:

while (from && from !== common) {

while (to && to !== common) {

They should’ve checked for alternate too.

gaearon added a commit to gaearon/react that referenced this issue Oct 9, 2017
gaearon added a commit to gaearon/react that referenced this issue Oct 9, 2017
@gaearon
Copy link
Collaborator

gaearon commented Oct 9, 2017

I have a fix in #11164.

gaearon added a commit that referenced this issue Oct 9, 2017
* Add a regression test for #10906

* Turn while conditions into breaks without changing the logic

This will be easier to follow when we add more code there.

* var => const/let

So that I can add a block scoped variable.

* Check alternates when comparing to the common ancestor

This is the actual bugfix.
@gaearon
Copy link
Collaborator

gaearon commented Nov 3, 2017

React 16.1.0-beta has been released. Please update react, react-dom, and react-test-renderer (if you use it) to this version and let us know if it solved the issue! We’d appreciate if you could test before Monday when we plan to get 16.1.0 out.

@Sherryer
Copy link
Author

Sherryer commented Nov 3, 2017

It works great, thank you!

@natpeng
Copy link

natpeng commented Nov 20, 2018

@cyan33 should you do event.stopPropagation(); in your enter function?

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

Successfully merging a pull request may close this issue.

6 participants