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

Error in console - Cannot read property 'capture' of null #23

Closed
ddtch opened this issue Feb 9, 2018 · 9 comments
Closed

Error in console - Cannot read property 'capture' of null #23

ddtch opened this issue Feb 9, 2018 · 9 comments
Assignees
Labels

Comments

@ddtch
Copy link

ddtch commented Feb 9, 2018

I imported 'default-passive-events' in one of my components where I use event listener and it was fine, but now when I navigate throw site, on one of the pages I got error like
image

I tried to remove all window and document listeners, to found a problem but that didn't helped. Any ideas || suggestions?

@FRSgit
Copy link
Collaborator

FRSgit commented Feb 12, 2018

Hey @ddtch,
Before anything - can you please try to create codepen where we can reproduce the error?

Also:

  • is "index.js" in attached error stack trace is pointing to default-passive-events package?
  • I see you use React - do you use any other third-party libraries?
  • I think it's worth mentioning that our package, because of it's intended purpose, is tweaking handling of passive events globally which means there is no difference if you include it inside single component or for whole app.

@FRSgit FRSgit self-assigned this Feb 12, 2018
@FRSgit
Copy link
Collaborator

FRSgit commented Feb 13, 2018

Okay, I figured out what's going on - in index.js:36 we check options variable only if it's typeof object and null is true value for such condition.

FRSgit pushed a commit that referenced this issue Feb 13, 2018
#23
Added additional check for null value.
@FRSgit FRSgit added bug and removed question labels Feb 13, 2018
@FRSgit
Copy link
Collaborator

FRSgit commented Feb 13, 2018

Okay, it should be fixed with commit f34ad72.
Before I push the newest version to npm can you please, @ddtch, recheck if it got fixed?
Just instead of npm add our package directly from github, e.g. with this command:
npm install https://github.com/zzarcon/default-passive-events.git

@ddtch
Copy link
Author

ddtch commented Feb 15, 2018

@FRSgit this helped and there is no error I reported, but no I got error. Here code from my component where I import passive events lib

class SomePage extends PureComponent {
	...
	componentDidMount() {
		...
		window.addEventListener('scroll', this.onScrollEvent);
	}

	componentWillUnmount() {
		window.removeEventListener('scroll', this.onScrollEvent);
	}

	onScrollEvent = () => {
		if(document.documentElement.scrollTop > 1200){
		    this.setState({showUpButton: true})
		} else {
		    this.setState({showUpButton: false})
		}
	};
	....
}

After I navigate to SomePage everything is fine. But when I pressed browser's 'back button' got error in console

warning.js:33 Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the SomePage component.

As I see for some reason eventListener actually not removes. So can you help me understand where the problem is?

@FRSgit
Copy link
Collaborator

FRSgit commented Feb 16, 2018

Okay, that one is connected with a way I've handled #19. Need another way to handle that one - I'm creating a wrapper around function passed to addEventListener, so it's not this.onScrollEvent anymore, but it's wrapper - that's why you later can't remove it properly (you try to remove listener this.onScrollEvent, but it won't work because our wrapper is actually the thing you should try to pass as a listener).

Give me some time to think about that, because now I don't have any good idea how to handle that one without regression on #19 (which I would like to avoid).

@ddtch
Copy link
Author

ddtch commented Feb 16, 2018

ok, on weekend I also will think about it.

@FRSgit
Copy link
Collaborator

FRSgit commented Feb 17, 2018

Also - need to there is need to write test checking if removeEventListener functionality works well.

@FRSgit
Copy link
Collaborator

FRSgit commented Mar 9, 2018

Sorry, was on holidays for last weeks. Today I've started working on the problem here WICG/interventions#63 - or rather on how properly find out if current listener is passive or not.

FRSgit pushed a commit that referenced this issue Apr 10, 2018
…), removed also connected utils & tests.

#24 - Since fix for #19 was removed that issue is no longer a case.

Added defer & async attributes for getting package's code in example from docs/index.html.
Added contributors property to package.json.
Updated dependencies.
@FRSgit
Copy link
Collaborator

FRSgit commented Apr 10, 2018

Unfortunately to bring important functionality of removingEventListener I needed to regress the package and remove fix for #19.

Problem is that "error message" caused by firing preventDefault in passive listener is not actually an error (under the hood it's just console.error), so it's not possible to catch & silence it.

In addition to what's above I've created issue on WHATWG repo - whatwg/dom#587 - let's see if anything will be done to cover our case.

Closing as fixed, rest of work is in hands of browser suppliers.

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

No branches or pull requests

2 participants