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

Ensure adapters and serializers are destroyed upon store destruction. #7020

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/-ember-data/node-tests/fixtures/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ module.exports = {
'(public) @ember-data/adapter MinimumAdapterInterface#shouldReloadAll [OPTIONAL]',
'(public) @ember-data/adapter MinimumAdapterInterface#shouldReloadRecord [OPTIONAL]',
'(public) @ember-data/adapter MinimumAdapterInterface#updateRecord',
'(public) @ember-data/adapter MinimumAdapterInterface#destroy [OPTIONAL]',
'(public) @ember-data/adapter RESTAdapter#coalesceFindRequests',
'(public) @ember-data/adapter RESTAdapter#createRecord',
'(public) @ember-data/adapter RESTAdapter#deleteRecord',
Expand Down
28 changes: 28 additions & 0 deletions packages/-ember-data/tests/integration/store/adapter-for-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { assign } from '@ember/polyfills';
import { run } from '@ember/runloop';

import { module, test } from 'qunit';

Expand Down Expand Up @@ -278,4 +279,31 @@ module('integration/store - adapterFor', function(hooks) {
);
assert.ok(jsonApiAdapter === adapter, 'We fell back to the -json-api adapter instance for the per-type adapter');
});

test('adapters are destroyed', async function(assert) {
let { owner } = this;
let didInstantiate = false;
let didDestroy = false;

class AppAdapter extends TestAdapter {
didInit() {
didInstantiate = true;
}

destroy() {
didDestroy = true;
}
}

owner.register('adapter:application', AppAdapter);

let adapter = store.adapterFor('application');

assert.ok(adapter instanceof AppAdapter, 'precond - We found the correct adapter');
assert.ok(didInstantiate, 'precond - We instantiated the adapter');

run(store, 'destroy');

assert.ok(didDestroy, 'adapter was destroyed');
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { assign } from '@ember/polyfills';
import { run } from '@ember/runloop';

import { module, test } from 'qunit';

Expand Down Expand Up @@ -454,4 +455,31 @@ module('integration/store - serializerFor', function(hooks) {
);
}
);

test('serializers are destroyed', async function(assert) {
let { owner } = this;
let didInstantiate = false;
let didDestroy = false;

class AppSerializer extends TestSerializer {
didInit() {
didInstantiate = true;
}

destroy() {
didDestroy = true;
}
}

owner.register('serializer:application', AppSerializer);

let serializer = store.serializerFor('application');

assert.ok(serializer instanceof AppSerializer, 'precond - We found the correct serializer');
assert.ok(didInstantiate, 'precond - We instantiated the serializer');

run(store, 'destroy');

assert.ok(didDestroy, 'serializer was destroyed');
});
});
23 changes: 19 additions & 4 deletions packages/store/addon/-private/system/core-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3487,14 +3487,29 @@ abstract class CoreStore extends Service {
}
}

destroy() {
// enqueue destruction of any adapters/serializers we have created
for (let adapterName in this._adapterCache) {
let adapter = this._adapterCache[adapterName];
if (typeof adapter.destroy === 'function') {
adapter.destroy();
}
}

for (let serializerName in this._serializerCache) {
let serializer = this._serializerCache[serializerName];
if (typeof serializer.destroy === 'function') {
serializer.destroy();
}
}

return super.destroy();
}

willDestroy() {
super.willDestroy();
this.recordArrayManager.destroy();

// Check if we need to null this out
// this._adapterCache = null;
// this._serializerCache = null;

identifierCacheFor(this).destroy();

this.unloadAll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,18 @@ interface Adapter {
* @return {boolean} true if the a new request for all records of the type in SnapshotRecordArray should be made in the background, false otherwise
*/
shouldBackgroundReloadAll?(store: Store, snapshotArray: SnapshotRecordArray): boolean;

/**
* In some situations the adapter may need to perform cleanup when destroyed,
* that cleanup can be done in `destroy`.
*
* If not implemented, the store does not inform the adapter of destruction.
*
* @method destroy [OPTIONAL]
* @public
* @optional
*/
destroy?(): void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you! <3

}

export default Adapter;