Skip to content

Commit

Permalink
fix: catch errors during didCommit in DEBUG (#8708)
Browse files Browse the repository at this point in the history
  • Loading branch information
runspired authored and jrjohnson committed Jul 28, 2023
1 parent 4bbc173 commit 34e90e0
Show file tree
Hide file tree
Showing 9 changed files with 498 additions and 28 deletions.
12 changes: 12 additions & 0 deletions packages/graph/src/-private/graph/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ export class Graph {
isReleasable(identifier: StableRecordIdentifier): boolean {
const relationships = this.identifiers.get(identifier);
if (!relationships) {
if (LOG_GRAPH) {
// eslint-disable-next-line no-console
console.log(`graph: RELEASABLE ${String(identifier)}`);
}
return true;
}
const keys = Object.keys(relationships);
Expand All @@ -213,9 +217,17 @@ export class Graph {
}
assert(`Expected a relationship`, relationship);
if (relationship.definition.inverseIsAsync) {
if (LOG_GRAPH) {
// eslint-disable-next-line no-console
console.log(`graph: <<NOT>> RELEASABLE ${String(identifier)}`);
}
return false;
}
}
if (LOG_GRAPH) {
// eslint-disable-next-line no-console
console.log(`graph: RELEASABLE ${String(identifier)}`);
}
return true;
}

Expand Down
4 changes: 4 additions & 0 deletions packages/model/src/-private/many-array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,10 @@ export default class RelatedCollection extends RecordArray {

return record;
}

destroy() {
super.destroy(false);
}
}
RelatedCollection.prototype.isAsync = false;
RelatedCollection.prototype.isPolymorphic = false;
Expand Down
41 changes: 33 additions & 8 deletions packages/store/src/-private/managers/record-array-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,15 @@ class RecordArrayManager {
declare store: Store;
declare isDestroying: boolean;
declare isDestroyed: boolean;
declare _set: Map<IdentifierArray, Set<StableRecordIdentifier>>;
declare _live: Map<string, IdentifierArray>;
declare _managed: Set<IdentifierArray>;
declare _pending: Map<IdentifierArray, ChangeSet>;
declare _identifiers: Map<StableRecordIdentifier, Set<Collection>>;
declare _staged: Map<string, ChangeSet>;
declare _subscription: UnsubscribeToken;
declare _keyedArrays: Map<string, Collection>;
declare _visibilitySet: Map<StableRecordIdentifier, boolean>;

constructor(options: { store: Store }) {
this.store = options.store;
Expand All @@ -97,13 +99,17 @@ class RecordArrayManager {
this._staged = new Map();
this._keyedArrays = new Map();
this._identifiers = new Map();
this._set = new Map();
this._visibilitySet = new Map();

this._subscription = this.store.notifications.subscribe(
'resource',
(identifier: StableRecordIdentifier, type: CacheOperation) => {
if (type === 'added') {
this._visibilitySet.set(identifier, true);
this.identifierAdded(identifier);
} else if (type === 'removed') {
this._visibilitySet.set(identifier, false);
this.identifierRemoved(identifier);
} else if (type === 'state') {
this.identifierChanged(identifier);
Expand All @@ -119,7 +125,7 @@ class RecordArrayManager {
return;
}

sync(array, pending);
sync(array, pending, this._set.get(array)!);
this._pending.delete(array);
}

Expand Down Expand Up @@ -154,6 +160,7 @@ class RecordArrayManager {
manager: this,
});
this._live.set(type, array);
this._set.set(array, new Set(identifiers));
}

return array;
Expand All @@ -178,6 +185,7 @@ class RecordArrayManager {
};
let array = new Collection(options);
this._managed.add(array);
this._set.set(array, new Set(options.identifiers || []));
if (config.identifiers) {
associate(this._identifiers, array, config.identifiers);
}
Expand Down Expand Up @@ -261,6 +269,7 @@ class RecordArrayManager {
const old = source.slice();
source.length = 0;
fastPush(source, identifiers);
this._set.set(array, new Set(identifiers));

notifyArray(array);
array.meta = payload.meta || null;
Expand Down Expand Up @@ -306,23 +315,32 @@ class RecordArrayManager {
identifierChanged(identifier: StableRecordIdentifier): void {
let newState = this.store._instanceCache.recordIsLoaded(identifier, true);

// if the change matches the most recent direct added/removed
// state, then we can ignore it
if (this._visibilitySet.get(identifier) === newState) {
return;
}

if (newState) {
this.identifierAdded(identifier);
} else {
this.identifierRemoved(identifier);
}
}

clear() {
this._live.forEach((array) => array.destroy());
this._managed.forEach((array) => array.destroy());
clear(isClear = true) {
this._live.forEach((array) => array.destroy(isClear));
this._managed.forEach((array) => array.destroy(isClear));
this._managed.clear();
this._identifiers.clear();
this._pending.clear();
this._set.forEach((set) => set.clear());
this._visibilitySet.clear();
}

destroy() {
this.isDestroying = true;
this.clear();
this.clear(false);
this._live.clear();
this.isDestroyed = true;
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
Expand Down Expand Up @@ -367,33 +385,40 @@ export function disassociateIdentifier(
}
}

function sync(array: IdentifierArray, changes: Map<StableRecordIdentifier, 'add' | 'del'>) {
function sync(
array: IdentifierArray,
changes: Map<StableRecordIdentifier, 'add' | 'del'>,
arraySet: Set<StableRecordIdentifier>
) {
let state = array[SOURCE];
const adds: StableRecordIdentifier[] = [];
const removes: StableRecordIdentifier[] = [];
changes.forEach((value, key) => {
if (value === 'add') {
// likely we want to keep a Set along-side
if (state.includes(key)) {
if (arraySet.has(key)) {
return;
}
adds.push(key);
arraySet.add(key);
} else {
if (state.includes(key)) {
if (arraySet.has(key)) {
removes.push(key);
}
}
});
if (removes.length) {
if (removes.length === state.length) {
state.length = 0;
arraySet.clear();
// changing the reference breaks the Proxy
// state = array[SOURCE] = [];
} else {
removes.forEach((i) => {
const index = state.indexOf(i);
if (index !== -1) {
state.splice(index, 1);
arraySet.delete(i);
}
});
}
Expand Down
10 changes: 5 additions & 5 deletions packages/store/src/-private/record-arrays/identifier-array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,13 @@ class IdentifierArray {
declare store: Store;
declare _manager: RecordArrayManager;

destroy() {
this.isDestroying = true;
destroy(clear: boolean) {
this.isDestroying = !clear;
// changing the reference breaks the Proxy
// this[SOURCE] = [];
this[SOURCE].length = 0;
this[NOTIFY]();
this.isDestroyed = true;
this.isDestroyed = !clear;
}

// length must be on self for proxied methods to work properly
Expand Down Expand Up @@ -540,8 +540,8 @@ export class Collection extends IdentifierArray {
return promise;
}

destroy() {
super.destroy();
destroy(clear: boolean) {
super.destroy(clear);
this._manager._managed.delete(this);
this._manager._pending.delete(this);
}
Expand Down
37 changes: 27 additions & 10 deletions packages/store/src/-private/store-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,17 +265,34 @@ class Store extends EmberObject {
_run(cb: () => void) {
assert(`EmberData should never encounter a nested run`, !this._cbs);
const _cbs: { coalesce?: () => void; sync?: () => void; notify?: () => void } = (this._cbs = {});
cb();
if (_cbs.coalesce) {
_cbs.coalesce();
}
if (_cbs.sync) {
_cbs.sync();
}
if (_cbs.notify) {
_cbs.notify();
if (DEBUG) {
try {
cb();
if (_cbs.coalesce) {
_cbs.coalesce();
}
if (_cbs.sync) {
_cbs.sync();
}
if (_cbs.notify) {
_cbs.notify();
}
} finally {
this._cbs = null;
}
} else {
cb();
if (_cbs.coalesce) {
_cbs.coalesce();
}
if (_cbs.sync) {
_cbs.sync();
}
if (_cbs.notify) {
_cbs.notify();
}
this._cbs = null;
}
this._cbs = null;
}
_join(cb: () => void): void {
if (this._cbs) {
Expand Down
10 changes: 5 additions & 5 deletions tests/main/tests/integration/record-array-manager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,26 +79,26 @@ module('integration/record_array_manager', function (hooks) {
let adapterPopulated = manager.createArray({ type: 'person', query });
let identifier = recordIdentifierFor(person);

assert.false(all.isDestroyed, 'initial: no calls to all.willDestroy');
assert.false(adapterPopulated.isDestroyed, 'initial: no calls to adapterPopulated.willDestroy');
assert.false(all.isDestroyed, 'initial: LiveArray is not destroyed');
assert.false(adapterPopulated.isDestroyed, 'initial: Collection is not destroyed');
assert.strictEqual(
manager._identifiers.get(identifier)?.size,
undefined,
'initial: expected the person to be a member of 0 AdapterPopulatedRecordArrays'
'initial: expected the person to be a member of 0 Collections'
);
assert.true(manager._live.has('person'), 'initial: we have a live array for person');

manager.destroy();
await settled();

assert.true(all.isDestroyed, 'all.willDestroy called once');
assert.true(all.isDestroyed, 'LiveArray is destroyed');
assert.false(manager._live.has('person'), 'no longer have a live array for person');
assert.strictEqual(
manager._identifiers.get(identifier)?.size,
undefined,
'expected the person to be a member of no recordArrays'
);
assert.true(adapterPopulated.isDestroyed, 'adapterPopulated.willDestroy called once');
assert.true(adapterPopulated.isDestroyed, 'Collection is destroyed');
});

test('#GH-4041 store#query AdapterPopulatedRecordArrays are removed from their managers instead of retained when #destroy is called', async function (assert) {
Expand Down
Loading

0 comments on commit 34e90e0

Please sign in to comment.