funkensturm / ember-local-storage

The addon provides a storageFor computed property that returns a proxy and persists the changes to localStorage or sessionStorage. It ships with an ember-data adapter.
https://www.funkensturm.com/ember-local-storage/
MIT License
218 stars 76 forks source link

Memory leak due to objects created in `storageFor` not being destroyed. #376

Closed Mikek2252 closed 8 months ago

Mikek2252 commented 8 months ago

The objects created in the storageFor helper and then stored in storages are not being destroyed after each test instance. This means the event listener is not removed (as willDestroy does not trigger) and is leaking the app container.

This can we seen by running the test suite of this addon and taking a memory snapshot.

fsmanuel commented 8 months ago

@Mikek2252 thanks for reporting! Do you use the Test Helpers:

import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { visit, currentURL } from '@ember/test-helpers';
import resetStorages from 'ember-local-storage/test-support/reset-storage';

module('basic acceptance test', function(hooks) {
  let hooks = setupApplicationTest(hooks);

  hooks.afterEach(function() {
    if (window.localStorage) {
      window.localStorage.clear();
    }
    if (window.sessionStorage) {
      window.sessionStorage.clear();
    }
    resetStorages();
  });

  test('can visit /', async function(assert) {
    await visit('/');
    assert.strictEqual(currentURL(), '/');
  });
});

If this does not fix the memory leak I'll need to investigate.

Mikek2252 commented 8 months ago

@fsmanuel Hi, yeah i am using resetStorages but the objects inside of storages are not destroyed. I think this is because the Object returned by storageFor is wrapped in a computed property so ember doesn't know it needs to destroy it. I have created a PR with a fix. You should be able to see the leak if you run the tests of ember-local-storage and do a snapshot. Also for clarification the leak only occurs on objects created by storageFor and the objects created in the adapter are destroyed.