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

Fix test failures in Ember 3.20: Run service teardown code in willDestroy() #241

Merged
merged 1 commit into from
Jul 14, 2020

Conversation

bendemboski
Copy link
Contributor

@bendemboski bendemboski commented Jul 10, 2020

Put the service teardown code in willDestroy() instead of destroy() so it runs at the expected time. destroy() schedules a call to willDestroy() on the run loop, so they happen at different times, and willDestroy() is the hook where teardown code is expected to live. Starting in Ember 3.20 having this code in destroy() can cause test teardown bugs if a modifier calls into the service to stop watching, because things happen in this order:

  1. Service's destroy() method is called (by the container)
  2. Modifiers' willRemove() hooks are called
  3. Service's willDestroy() hook is called

So the service's teardown code needs to live in willDestroy() or else it will have already torn down by step (2) and calls into it from modifier teardown code can cause errors.

Note that I did not add any new tests as part of this PR because the existing tests are failing in the beta and (I'm pretty sure) canary scenarios, so this change already has test coverage.

Closes #240

Put the service teardown code in willDestroy() instead of destroy() so it runs at the expected time. destroy() schedules a call to willDestroy() on the run loop, so they happen at different times, and willDestroy() is the hook where teardown code is expected to live. Starting in Ember 3.20 having this code in destroy() can cause test teardown bugs if a modifier calls into the service to stop watching, because things happen in this order:

1. Service's destroy() method is called (by the container)
2. Modifiers' willRemove() hooks are called
3. Service's willDestroy() hook is called

So the service's teardown code needs to live in willDestroy() or else it will have already torn down by step (2) and calls into it from component/modifier teardown code can cause errors.
@makepanic
Copy link

Can confirm, this change fixes our build breaking with ember-in-viewport and [email protected]

@snewcomer snewcomer added the bug label Jul 14, 2020
Copy link
Collaborator

@snewcomer snewcomer left a comment

Choose a reason for hiding this comment

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

destroy() schedules a call to willDestroy() on the run loop

If it schedules it in a runloop, does that mean it happens in a specific queue that is still outside of when willDestroy happens?

@bendemboski
Copy link
Contributor Author

The destroy() method sets the isDestroying flag and schedules a callback on the destroy queue, and that callback calls the willDestroy hook (if implemented) and then does the actual work to destroy the object. willDestroy is where we're meant to do custom teardown work.

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

Successfully merging this pull request may close these issues.

Test failure in Ember 3.20
3 participants