Skip to content

Commit 662ce74

Browse files
jrjohnsonrunspired
andauthored
Backport into release (#8750)
* fix: @ember-data/debug should declare its peer-dependency on @ember-data/store (#8703) * fix: de-dupe coalescing when includes or adapterOptions is present but still use findRecord (#8704) * fix: make implicit relationship teardown following delete of related record safe (#8705) * fix: catch errors during didCommit in DEBUG (#8708) --------- Co-authored-by: Chris Thoburn <runspired@users.noreply.github.com>
1 parent 628fa59 commit 662ce74

File tree

14 files changed

+2049
-115
lines changed

14 files changed

+2049
-115
lines changed

packages/debug/package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
"directories": {},
1616
"scripts": {},
1717
"peerDependencies": {
18-
"@ember/string": "^3.1.1"
18+
"@ember/string": "^3.1.1",
19+
"@ember-data/store": "workspace:4.12.2"
1920
},
2021
"dependenciesMeta": {
2122
"@ember-data/private-build-infra": {

packages/graph/src/-private/graph/graph.ts

+17
Original file line numberDiff line numberDiff line change
@@ -199,16 +199,33 @@ export class Graph {
199199
isReleasable(identifier: StableRecordIdentifier): boolean {
200200
const relationships = this.identifiers.get(identifier);
201201
if (!relationships) {
202+
if (LOG_GRAPH) {
203+
// eslint-disable-next-line no-console
204+
console.log(`graph: RELEASABLE ${String(identifier)}`);
205+
}
202206
return true;
203207
}
204208
const keys = Object.keys(relationships);
205209
for (let i = 0; i < keys.length; i++) {
206210
const relationship: RelationshipEdge = relationships[keys[i]];
211+
// account for previously unloaded relationships
212+
// typically from a prior deletion of a record that pointed to this one implicitly
213+
if (relationship === undefined) {
214+
continue;
215+
}
207216
assert(`Expected a relationship`, relationship);
208217
if (relationship.definition.inverseIsAsync) {
218+
if (LOG_GRAPH) {
219+
// eslint-disable-next-line no-console
220+
console.log(`graph: <<NOT>> RELEASABLE ${String(identifier)}`);
221+
}
209222
return false;
210223
}
211224
}
225+
if (LOG_GRAPH) {
226+
// eslint-disable-next-line no-console
227+
console.log(`graph: RELEASABLE ${String(identifier)}`);
228+
}
212229
return true;
213230
}
214231

packages/legacy-compat/src/legacy-network-handler/fetch-manager.ts

+103-40
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ interface PendingFetchItem {
4444
resolver: Deferred<any>;
4545
options: FindOptions;
4646
trace?: unknown;
47-
promise: Promise<StableRecordIdentifier>;
47+
promise: Promise<StableExistingRecordIdentifier>;
4848
}
4949

5050
interface PendingSaveItem {
@@ -60,7 +60,7 @@ export default class FetchManager {
6060
declare isDestroyed: boolean;
6161
declare requestCache: RequestStateService;
6262
// fetches pending in the runloop, waiting to be coalesced
63-
declare _pendingFetch: Map<string, PendingFetchItem[]>;
63+
declare _pendingFetch: Map<string, Map<StableExistingRecordIdentifier, PendingFetchItem[]>>;
6464
declare _store: Store;
6565

6666
constructor(store: Store) {
@@ -114,7 +114,7 @@ export default class FetchManager {
114114
identifier: StableExistingRecordIdentifier,
115115
options: FindOptions,
116116
request: StoreRequestInfo
117-
): Promise<StableRecordIdentifier> {
117+
): Promise<StableExistingRecordIdentifier> {
118118
let query: FindRecordQuery = {
119119
op: 'findRecord',
120120
recordIdentifier: identifier,
@@ -193,13 +193,21 @@ export default class FetchManager {
193193
});
194194
}
195195

196-
let fetches = this._pendingFetch;
196+
let fetchesByType = this._pendingFetch;
197+
let fetchesById = fetchesByType.get(modelName);
197198

198-
if (!fetches.has(modelName)) {
199-
fetches.set(modelName, []);
199+
if (!fetchesById) {
200+
fetchesById = new Map();
201+
fetchesByType.set(modelName, fetchesById);
200202
}
201203

202-
(fetches.get(modelName) as PendingFetchItem[]).push(pendingFetchItem);
204+
let requestsForIdentifier = fetchesById.get(identifier);
205+
if (!requestsForIdentifier) {
206+
requestsForIdentifier = [];
207+
fetchesById.set(identifier, requestsForIdentifier);
208+
}
209+
210+
requestsForIdentifier.push(pendingFetchItem);
203211

204212
if (TESTING) {
205213
if (!request.disableTestWaiter) {
@@ -214,14 +222,12 @@ export default class FetchManager {
214222
return promise;
215223
}
216224

217-
getPendingFetch(identifier: StableRecordIdentifier, options: FindOptions) {
218-
let pendingFetches = this._pendingFetch.get(identifier.type);
225+
getPendingFetch(identifier: StableExistingRecordIdentifier, options: FindOptions) {
226+
let pendingFetches = this._pendingFetch.get(identifier.type)?.get(identifier);
219227

220228
// We already have a pending fetch for this
221229
if (pendingFetches) {
222-
let matchingPendingFetch = pendingFetches.find(
223-
(fetch) => fetch.identifier === identifier && isSameRequest(options, fetch.options)
224-
);
230+
let matchingPendingFetch = pendingFetches.find((fetch) => isSameRequest(options, fetch.options));
225231
if (matchingPendingFetch) {
226232
return matchingPendingFetch.promise;
227233
}
@@ -239,15 +245,15 @@ export default class FetchManager {
239245
}
240246

241247
fetchDataIfNeededForIdentifier(
242-
identifier: StableRecordIdentifier,
248+
identifier: StableExistingRecordIdentifier,
243249
options: FindOptions = {},
244250
request: StoreRequestInfo
245-
): Promise<StableRecordIdentifier> {
251+
): Promise<StableExistingRecordIdentifier> {
246252
// pre-loading will change the isEmpty value
247253
const isEmpty = _isEmpty(this._store._instanceCache, identifier);
248254
const isLoading = _isLoading(this._store._instanceCache, identifier);
249255

250-
let promise: Promise<StableRecordIdentifier>;
256+
let promise: Promise<StableExistingRecordIdentifier>;
251257
if (isEmpty) {
252258
assertIdentifierHasId(identifier);
253259

@@ -296,12 +302,47 @@ function _isLoading(cache: InstanceCache, identifier: StableRecordIdentifier): b
296302
);
297303
}
298304

305+
function includesSatisfies(current: undefined | string | string[], existing: undefined | string | string[]): boolean {
306+
// if we have no includes we are good
307+
if (!current?.length) {
308+
return true;
309+
}
310+
311+
// if we are here we have includes,
312+
// and if existing has no includes then we will need a new request
313+
if (!existing?.length) {
314+
return false;
315+
}
316+
317+
const arrCurrent = (Array.isArray(current) ? current : current.split(',')).sort();
318+
const arrExisting = (Array.isArray(existing) ? existing : existing.split(',')).sort();
319+
320+
// includes are identical
321+
if (arrCurrent.join(',') === arrExisting.join(',')) {
322+
return true;
323+
}
324+
325+
// if all of current includes are in existing includes then we are good
326+
// so if we find one that is not in existing then we need a new request
327+
for (let i = 0; i < arrCurrent.length; i++) {
328+
if (!arrExisting.includes(arrCurrent[i])) {
329+
return false;
330+
}
331+
}
332+
333+
return true;
334+
}
335+
336+
function optionsSatisfies(current: object | undefined, existing: object | undefined): boolean {
337+
return !current || current === existing || Object.keys(current).length === 0;
338+
}
339+
299340
// this function helps resolve whether we have a pending request that we should use instead
300341
function isSameRequest(options: FindOptions = {}, existingOptions: FindOptions = {}) {
301-
let includedMatches = !options.include || options.include === existingOptions.include;
302-
let adapterOptionsMatches = options.adapterOptions === existingOptions.adapterOptions;
303-
304-
return includedMatches && adapterOptionsMatches;
342+
return (
343+
optionsSatisfies(options.adapterOptions, existingOptions.adapterOptions) &&
344+
includesSatisfies(options.include, existingOptions.include)
345+
);
305346
}
306347

307348
function _findMany(
@@ -499,35 +540,57 @@ function _processCoalescedGroup(
499540
}
500541
}
501542

502-
function _flushPendingFetchForType(store: Store, pendingFetchItems: PendingFetchItem[], modelName: string) {
543+
function _flushPendingFetchForType(
544+
store: Store,
545+
pendingFetchMap: Map<StableExistingRecordIdentifier, PendingFetchItem[]>,
546+
modelName: string
547+
) {
503548
let adapter = store.adapterFor(modelName);
504549
let shouldCoalesce = !!adapter.findMany && adapter.coalesceFindRequests;
505-
let totalItems = pendingFetchItems.length;
506550

507551
if (shouldCoalesce) {
508-
let snapshots = new Array<Snapshot>(totalItems);
509-
let fetchMap = new Map<Snapshot, PendingFetchItem>();
510-
for (let i = 0; i < totalItems; i++) {
511-
let fetchItem = pendingFetchItems[i];
512-
snapshots[i] = store._fetchManager.createSnapshot(fetchItem.identifier, fetchItem.options);
513-
fetchMap.set(snapshots[i], fetchItem);
514-
}
552+
const pendingFetchItems: PendingFetchItem[] = [];
553+
pendingFetchMap.forEach((requestsForIdentifier, identifier) => {
554+
if (requestsForIdentifier.length > 1) {
555+
return;
556+
}
515557

516-
let groups: Snapshot[][];
517-
if (adapter.groupRecordsForFindMany) {
518-
groups = adapter.groupRecordsForFindMany(store, snapshots);
519-
} else {
520-
groups = [snapshots];
521-
}
558+
// remove this entry from the map so it's not processed again
559+
pendingFetchMap.delete(identifier);
560+
pendingFetchItems.push(requestsForIdentifier[0]);
561+
});
522562

523-
for (let i = 0, l = groups.length; i < l; i++) {
524-
_processCoalescedGroup(store, fetchMap, groups[i], adapter, modelName);
525-
}
526-
} else {
527-
for (let i = 0; i < totalItems; i++) {
528-
void _fetchRecord(store, adapter, pendingFetchItems[i]);
563+
let totalItems = pendingFetchItems.length;
564+
565+
if (totalItems > 1) {
566+
let snapshots = new Array<Snapshot>(totalItems);
567+
let fetchMap = new Map<Snapshot, PendingFetchItem>();
568+
for (let i = 0; i < totalItems; i++) {
569+
let fetchItem = pendingFetchItems[i];
570+
snapshots[i] = store._fetchManager.createSnapshot(fetchItem.identifier, fetchItem.options);
571+
fetchMap.set(snapshots[i], fetchItem);
572+
}
573+
574+
let groups: Snapshot[][];
575+
if (adapter.groupRecordsForFindMany) {
576+
groups = adapter.groupRecordsForFindMany(store, snapshots);
577+
} else {
578+
groups = [snapshots];
579+
}
580+
581+
for (let i = 0, l = groups.length; i < l; i++) {
582+
_processCoalescedGroup(store, fetchMap, groups[i], adapter, modelName);
583+
}
584+
} else if (totalItems === 1) {
585+
void _fetchRecord(store, adapter, pendingFetchItems[0]);
529586
}
530587
}
588+
589+
pendingFetchMap.forEach((pendingFetchItems) => {
590+
pendingFetchItems.forEach((pendingFetchItem) => {
591+
void _fetchRecord(store, adapter, pendingFetchItem);
592+
});
593+
});
531594
}
532595

533596
function _flushPendingSave(store: Store, pending: PendingSaveItem) {

packages/legacy-compat/src/legacy-network-handler/legacy-network-handler.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ function findBelongsTo<T>(context: StoreRequestContext): Promise<T> {
9797
const identifier = identifiers?.[0];
9898

9999
// short circuit if we are already loading
100-
let pendingRequest = identifier && store._fetchManager.getPendingFetch(identifier, options);
100+
let pendingRequest =
101+
identifier && store._fetchManager.getPendingFetch(identifier as StableExistingRecordIdentifier, options);
101102
if (pendingRequest) {
102103
return pendingRequest as Promise<T>;
103104
}

packages/model/src/-private/many-array.ts

+4
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,10 @@ export default class RelatedCollection extends RecordArray {
343343

344344
return record;
345345
}
346+
347+
destroy() {
348+
super.destroy(false);
349+
}
346350
}
347351
RelatedCollection.prototype.isAsync = false;
348352
RelatedCollection.prototype.isPolymorphic = false;

0 commit comments

Comments
 (0)