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

Bugfix - send action on destroy + memory leaks #138

Merged
merged 17 commits into from
Apr 6, 2018

Conversation

snewcomer
Copy link
Collaborator

@snewcomer snewcomer commented Mar 30, 2018

We want to still send the action didEnterViewport/didExitViewport (thanks Nathan H 🏅) even though the component is being torn down. seting state is simply around for the next onIntersection, which makes it irrelevant in this context.

close #141

@snewcomer snewcomer force-pushed the bugfix/send-action-on-destroy branch from 64595c5 to 14a4325 Compare March 30, 2018 17:22
@snewcomer snewcomer force-pushed the bugfix/send-action-on-destroy branch from 1c0c1e5 to 397cc15 Compare March 30, 2018 20:28
@@ -20,7 +20,6 @@ module('Acceptance | infinity-scrollable', function(hooks) {
await waitFor('.infinity-scrollable.inactive');

assert.equal(findAll('.infinity-svg').length, 20);
assert.equal(findAll('.infinity-scrollable.inactive').length, 1, 'component is inactive after fetching more data');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a race condition. For one, this fails randomly on diff builds. Second, it wouldn't proceed from the await waitFor unless it didn't find it as inactive.

Passes consistently locally; however, not up on CI.

@snewcomer snewcomer changed the title Bugfix/send action on destroy Bugfix - send action on destroy + memory leaks Apr 5, 2018
@snewcomer snewcomer mentioned this pull request Apr 5, 2018
elem.addEventListener('scroll', () => {
this._debouncedEventHandler('_triggerDidScrollDirection', elem, sensitivity);
});
this._debouncedEventHandler = this._debouncedEvent.bind(this, '_triggerDidScrollDirection', elem, sensitivity);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way I guess would be =>

this._debouncedEventClosure = (e => this._debouncedEventHandler('_triggerDidScrollDirection', elem, sensitivity));

@snewcomer snewcomer merged commit e98d3b2 into master Apr 6, 2018
@snewcomer snewcomer deleted the bugfix/send-action-on-destroy branch April 6, 2018 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leak: scroll event listener not being removed
1 participant