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

Uncaught Invariant Violation: Must be mounted to trap events when replacing an image with a link to an image #6371

Closed
aaronjensen opened this issue Mar 29, 2016 · 28 comments

Comments

@aaronjensen
Copy link

I'm having a hard time narrowing a repro down for this and there seem to be a lot of required moving parts, so I'll try to give as much information as I can.

Relevant snippet of package.json:

    "react": "15.0.0-rc.2", // repros w/ 0.14.7 as well
    "react-dom": "15.0.0-rc.2",
    "react-redux": "4.0.6",
    "react-router": "2.0.0-rc5",
    "react-router-redux": "4.0.0-rc.1",
    "redux": "3.2.1",

Our routes look something like this:

      <Route component={App}>
        <Route component={LayoutNoFooter}>
          <Route component={Page2} />
        </Route>
        <Route component={Layout}>
          <Route component={Page1} />
        </Route>
      </Route>

LayoutNoFooter simply wraps Layout and passes an additional prop in.

I have this component in the Header, which is rendered by Layout: https://gist.github.com/aaronjensen/86c9643ecc53ad4e9006

Header is a react-redux connected component.

Page1 has a componentWillUnmount that dispatches an action that ultimately toggles searchMode in HeaderLogo.

When I go from Page1 to Page2 the invariant violation fires.

If I setTimeout the action dispatch in componentWillUnmount it appears to work fine. If I replace LayoutNoFooter with Layout it appears to work fine.

The invariant violation is on the img tag of the HeaderLogo

stack

I've tried building a repro in codepen, with a similar structure setState calls to simulate react-router and redux, but I suspect they are doing more that would require me to actually build a repro w/ those technologies. If that would be helpful, I can work on it over time and hopefully repro it. Maybe there's enough info here, however. Thanks!

@gaearon gaearon added Component: Events Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Mar 29, 2016
@zpao
Copy link
Member

zpao commented Mar 29, 2016

This trapping code got moved around a bit in 15, can you paste the stack for that repro? A consistent repro we can run would be helpful, so I appreciate you trying, let us know if you do get it working.

The high level assumption here - we have some special event code for some events which don't bubble on some dom nodes. In this case it's the img in the header and likely a load (or error) event. It's possible the event is firing when componentWillUnmount is happening (which is in turn triggering a rerender and removing the img), but I would have thought that (a) this happens outside the current event loop and (b) the node is removed and event listener removed before this could happen. This unmount, setState handling code is pretty confusing though. Perhaps @spicyj knows what's up.

@sophiebits
Copy link
Collaborator

I don't know offhand. A repro case would be very helpful – you shouldn't ever see that error.

@aaronjensen
Copy link
Author

@spicyj nailing it down has been tough, but I can keep trying when I get time. I'd be available for remote debugging if that'd work for you as well.

@zpao I'll get the other stack trace when I get a chance as well

@Dattaya
Copy link

Dattaya commented Apr 5, 2016

I have this warning as well. I use:

+-- react@0.14.7
+-- react-router@2.0.1
`-- redux@3.3.1

In my case it happens when I'm transitioning from one page of the same component to another and dispatching an action in componentWillReceiveProps. If I don't dispatch it, there is no error. For the time being I've fixed it by putting the dispatch inside setTimeout(..., 0). Since you're saying that I shouldn't even see this error, I'll see if I can extract that failing code into a repository or a gist with all necessary dependencies (react-router, redux) if that's fine with you.

@aaronjensen
Copy link
Author

@Dattaya that would be awesome, I haven't had time to do this myself. Thanks!

@Dattaya
Copy link

Dattaya commented Apr 14, 2016

I'm not sure my case is worth investigating, the error happens only when both redux-devtools and react-router-redux present. But I said if I could, I would provide an example, so to hold on to my promise here it goes anyway.
Steps to reproduce an error:

Now in the console you should see an error: react-with-addons.js:20632 Uncaught Invariant Violation: Must be mounted to trap events

@jimfb
Copy link
Contributor

jimfb commented Apr 14, 2016

@gaearon Sounds like this might be redux related, especially if it requires the redux devtools extension be installed. Should we move this over to that repository?

@grahaml
Copy link

grahaml commented Apr 14, 2016

I am receiving the same error in a somewhat similar context. I am trying to push a route on a page before a component is mounted. Inside componentWillMount I am trying to check the props to validate whether or not this component should even be mounted. If it shouldn't be mounted (read: if an unauthenticated user is trying to load the account page, that component should not even mount, and the user should be redirected to the login page).

If I try the same logic inside componentDidMount it works as expected.

Example:

class AccountPage extends React.Component {
  componentWillMount() {
    if (!this.props.account.isLoggedIn) {
      this.context.router.push('/login'); // Throws invariant violation "Must be mounted to trap events"
    }
  }
}

package.json has

"react": "0.14.7",
"react-redux": "4.4.1",
"react-router": "2.0.1",
"redux": "^3.3.1",

@sophiebits
Copy link
Collaborator

@jimfb It should be impossible to trigger this error in React, so it is a React bug even if all the repro cases use Redux.

@jimfb
Copy link
Contributor

jimfb commented Apr 14, 2016

@spicyj Ah, yeah, probably true. I have no idea what the redux devtools extension is doing, but it's entirely conceivable that they're reaching into internals, in which case, it would not be a React bug. If the issue doesn't repro without the redux-devtools (which @Dattaya's comment seemed to imply) then that would seem to imply it might not be a React bug. But yeah, that's why I asked @gaearon.

@gaearon
Copy link
Collaborator

gaearon commented Apr 14, 2016

Redux DevTools don’t touch React internals, it’s only userland code.

@gaearon
Copy link
Collaborator

gaearon commented Apr 14, 2016

I can’t vouch for the Chrome extension though.
Maybe @zalmoxisus has some insight into this.

@jimfb
Copy link
Contributor

jimfb commented Apr 14, 2016

Yes, sorry for any ambiguity, I was referring to the chrome extension.

@zalmoxisus
Copy link

zalmoxisus commented Apr 15, 2016

@gaearon, that's strange. In the extension's injected script we don't use React at all, except the fact that it is imported from redux-devtools.

I just checked the example above and the error appears also when the extension is disabled (@Dattaya, could you confirm?). So, it doesn't seem to be related to Redux DevTools or the Chrome extension.

zalmoxisus added a commit to zalmoxisus/redux-devtools-extension that referenced this issue Apr 15, 2016
@pke
Copy link

pke commented Apr 15, 2016

@zalmoxisus I can confirm that the error also appears when the extension is disabled. Using react 15.0.0

@Dattaya
Copy link

Dattaya commented Apr 15, 2016

@zalmoxisus, that's correct, sorry about the confusion, I might have looked in the wrong console, when trying to replicate it in my project without dev tools. It's completely unrelated to Dev Tools.

@stream7
Copy link

stream7 commented Apr 20, 2016

I'm having the same problem and after a bit of debugging I see that callbacks queued using transaction.getReactMountReady().enqueue e.g. trapBubbledEventsLocal or putListener sometimes are executed even after the instance has been unmounted. Is this normal?

The actual error message is a bit different in each case but it occurs in both 0.14.8 and 15.0.1.

I'm using the same stack and the weird part is that this error occurs when I click on the react-router's Link but it doesn't if I use the browsers back/forward buttons.

Also confirming that it's completely unrelated to Dev Tools.

Adding another stack trace. Note that in my case it is a button element.

caterpillar_assets_360

Related #6538

@chelzwa
Copy link

chelzwa commented Apr 20, 2016

We're seeing the same stack trace as @stream7. We only see this error if the component which is unmounted on route change contains an onClick listener, and only if navigating via react-router Link, rather than browser back/forward buttons. If we unmount the component prior to route change, the error does not occur.

"react": "15.0.1",
"react-dom": "15.0.1",
"react-redux": "4.4.1",
"react-router": "2.3.0",
"redux": "3.3.1",

We also see this error in react & react-dom 0.14.8 or react-router 2.0.1.

@sophiebits
Copy link
Collaborator

@Dattaya Thanks for posting the repro case. Hopefully one of us can look into this.

@chelzwa
Copy link

chelzwa commented Apr 20, 2016

Here's an additional codepen showing the cannot read property 'addEventListener' of null error.

http://codepen.io/chelzwa/pen/JXZeOL?editors=1010

Steps to reproduce:

  1. Add any number of alerts
  2. Switch pages

Although my specific error is different than @Dattaya's, it seems likely to be related to the same cause, as I also found that this happens only if I include react-router-redux in the project.
Switching <Router history={appHistory}> to <Router history={browserHistory}> and commenting out the line const appHistory = syncHistoryWithStore(browserHistory, store); fixes the issue.

@gaearon
Copy link
Collaborator

gaearon commented Apr 21, 2016

Apparently both inst._rootNodeID and inst._nativeNode are null the second time it is called while handling the button click. This is why ReactDOMComponentTree can’t find it.

Not sure what this means though.

@stream7
Copy link

stream7 commented Apr 21, 2016

callbacks queued using transaction.getReactMountReady().enqueue e.g. trapBubbledEventsLocal or putListener sometimes are executed even after the instance has been unmounted.

@gaearon @spicyj do you know if this is expected behaviour? It sounds like it isn't but if we don't know this for sure we can't use it to debug further.

@jimfb
Copy link
Contributor

jimfb commented Apr 22, 2016

Minimal repro thusfar:

var callbacks = [];
function emitChange() {
  callbacks.forEach(c => c());
}

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = { showChild: true };
  }
  componentDidMount() {
    this.setState({ showChild: false });
  }
  render() {
    return (
      <div>
        <ForceUpdatesOnChange />
        {this.state.showChild && <EmitsChangeOnUnmount />}
      </div>
    );
  }
}

class EmitsChangeOnUnmount extends React.Component {
  componentWillUnmount() {
    emitChange();
  }
  render() {
    return null;
  }
}

class ForceUpdatesOnChange extends React.Component {
  componentDidMount() {
    this.onChange = () => this.forceUpdate();
    this.onChange()
    callbacks.push(this.onChange);
  }
  componentWillUnmount() {
    callbacks = callbacks.filter(c => c !== this.onChange)
  }
  render() {
    return (
      <div key={Math.random()} onClick={function(){}} />
    )
  }
}

ReactDOM.render(<App />, document.getElementById('container'));

Thanks Dan for helping reduce!

@sophiebits
Copy link
Collaborator

#2410 is a known problem which might cause this but otherwise that should only happen if something throws an exception (and maybe not even then).

@jimfb
Copy link
Contributor

jimfb commented Apr 22, 2016

@spicyj No, this is almost certainly caused by a variant of #3298

@jimfb
Copy link
Contributor

jimfb commented Apr 22, 2016

But it's not an exact duplicate, so I don't want to close this one out as a duplicate. In this case, it's something to the effect of: redux setting state on a component after/while react-router is unmounting it (or its parent). But it's a very similar phenomenon that occurs due to an unmount higher up in the tree within the same transaction.

@gioacostax
Copy link

Well, I have the same problem, y tried removing Dev Tools and it works.... so...

@gaearon
Copy link
Collaborator

gaearon commented Apr 24, 2016

Note: this is exactly the same as #6538. The message depends on what you render (#6538 (comment)), but the underlying issue is explained in #6538 (comment).

@gaearon gaearon added Component: Batching Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Apr 24, 2016
sophiebits added a commit to sophiebits/react that referenced this issue Apr 29, 2016
Fixes facebook#2410. Fixes facebook#6371. Fixes facebook#6538.

I also manually tested the codepen in facebook#3762 and verified it now works.
sophiebits added a commit to sophiebits/react that referenced this issue Apr 29, 2016
Fixes facebook#2410. Fixes facebook#6371. Fixes facebook#6538.

I also manually tested the codepen in facebook#3762 and verified it now works.
sophiebits added a commit that referenced this issue Apr 29, 2016
Fixes #2410. Fixes #6371. Fixes #6538.

I also manually tested the codepen in #3762 and verified it now works.
zpao pushed a commit that referenced this issue May 10, 2016
Fixes #2410. Fixes #6371. Fixes #6538.

I also manually tested the codepen in #3762 and verified it now works.
(cherry picked from commit c1e3f7e)
HadesHappy added a commit to HadesHappy/redux-devtools-extension that referenced this issue Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

13 participants