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

Polyfill QUnit memory leak prevention. #16620

Merged
merged 1 commit into from May 7, 2018

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented May 7, 2018

This polyfills the changes from qunitjs/qunit#1279 to older versions of QUnit.


Prior to the changes in this PR, all module and test callbacks are retained (forever). This may not seem significant, but as folks use closure scope to store data across tests (which is very common).

For example, prior to the changes in this PR the following would retain the local variable:

QUnit.module('top', function(hooks) {
  let largeThing;

  hooks.beforeEach(function() {
    largeThing = new LargeThing();
  });

  hooks.afterEach(function() {
    largeThing.destroy();
  });

  test('something that uses largeThing', function(assert) {
    // ...snip...
    largeThing.someMethod();
  });
});

This polyfills the changes from qunitjs/qunit#1279 to older versions of QUnit.

---

Prior to the changes in this PR, all module and test callbacks are retained (forever). This may not seem significant, but as folks use closure scope to store data across tests (which is very common).

For example, prior to the changes in this PR the following would retain the local variable:

```js
QUnit.module('top', function(hooks) {
  let largeThing;

  hooks.beforeEach(function() {
    largeThing = new LargeThing();
  });

  hooks.afterEach(function() {
    largeThing.destroy();
  });

  test('something that uses largeThing', function(assert) {
    // ...snip...
    largeThing.someMethod();
  });
});
```
@stefanpenner stefanpenner merged commit db31ac4 into emberjs:master May 7, 2018
@rwjblue rwjblue deleted the free-memory-from-test-hooks branch May 7, 2018 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants