Skip to content

Commit

Permalink
fix(incremental): fix logic around selecting id/subPath (#3987)
Browse files Browse the repository at this point in the history
current loop assumes that the first analyzed DeferredFragmentRecord has been released as pending and thereofre has an `id`, which happens to be the case in this scenario, but is not reliable, uncovered by #3982.

Demonstrates the peril of `!` which might point to requiring additional types, but we can leave that for a different PR.

Another option is to include an `invariant` call -- that should be unnecessary, but would indeed be safer.
  • Loading branch information
yaacovCR authored Nov 9, 2023
1 parent 2e29180 commit e081838
Showing 1 changed file with 14 additions and 14 deletions.
28 changes: 14 additions & 14 deletions src/execution/IncrementalPublisher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,29 +609,29 @@ export class IncrementalPublisher {
deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord,
): IncrementalDeferResult {
const { data, deferredFragmentRecords } = deferredGroupedFieldSetRecord;
let maxLength = deferredFragmentRecords[0].path.length;
let maxIndex = 0;
for (let i = 1; i < deferredFragmentRecords.length; i++) {
const deferredFragmentRecord = deferredFragmentRecords[i];
let maxLength: number | undefined;
let idWithLongestPath: string | undefined;
for (const deferredFragmentRecord of deferredFragmentRecords) {
const id = deferredFragmentRecord.id;
if (id === undefined) {
continue;
}
const length = deferredFragmentRecord.path.length;
if (length > maxLength) {
if (maxLength === undefined || length > maxLength) {
maxLength = length;
maxIndex = i;
idWithLongestPath = id;
}
}
const recordWithLongestPath = deferredFragmentRecords[maxIndex];
const longestPath = recordWithLongestPath.path;
const subPath = deferredGroupedFieldSetRecord.path.slice(
longestPath.length,
);
const id = recordWithLongestPath.id;
const subPath = deferredGroupedFieldSetRecord.path.slice(maxLength);
const incrementalDeferResult: IncrementalDeferResult = {
// safe because `data``is always defined when the record is completed
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
data: data!,
// safe because `id` is defined once the fragment has been released as pending
// safe because `id` is always defined once the fragment has been released
// as pending and at least one fragment has been completed, so must have been
// released as pending
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
id: id!,
id: idWithLongestPath!,
};

if (subPath.length > 0) {
Expand Down

0 comments on commit e081838

Please sign in to comment.