-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Revert optimistic rendering on negative response #7541
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,11 @@ import { useState } from 'react'; | |
import { v4 } from 'uuid'; | ||
|
||
import { triggerCreateRecordsOptimisticEffect } from '@/apollo/optimistic-effect/utils/triggerCreateRecordsOptimisticEffect'; | ||
import { triggerDeleteRecordsOptimisticEffect } from '@/apollo/optimistic-effect/utils/triggerDeleteRecordsOptimisticEffect'; | ||
import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; | ||
import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; | ||
import { useCreateOneRecordInCache } from '@/object-record/cache/hooks/useCreateOneRecordInCache'; | ||
import { deleteRecordFromCache } from '@/object-record/cache/utils/deleteRecordFromCache'; | ||
import { getObjectTypename } from '@/object-record/cache/utils/getObjectTypename'; | ||
import { RecordGqlOperationGqlRecordFields } from '@/object-record/graphql/types/RecordGqlOperationGqlRecordFields'; | ||
import { generateDepthOneRecordGqlFields } from '@/object-record/graphql/utils/generateDepthOneRecordGqlFields'; | ||
|
@@ -85,27 +87,49 @@ export const useCreateOneRecord = < | |
const mutationResponseField = | ||
getCreateOneRecordMutationResponseField(objectNameSingular); | ||
|
||
const createdObject = await apolloClient.mutate({ | ||
mutation: createOneRecordMutation, | ||
variables: { | ||
input: sanitizedInput, | ||
}, | ||
update: (cache, { data }) => { | ||
const record = data?.[mutationResponseField]; | ||
|
||
if (!record || skipPostOptmisticEffect) return; | ||
const createdObject = await apolloClient | ||
.mutate({ | ||
mutation: createOneRecordMutation, | ||
variables: { | ||
input: sanitizedInput, | ||
}, | ||
update: (cache, { data }) => { | ||
const record = data?.[mutationResponseField]; | ||
|
||
if (!record || skipPostOptmisticEffect) return; | ||
|
||
triggerCreateRecordsOptimisticEffect({ | ||
cache, | ||
objectMetadataItem, | ||
recordsToCreate: [record], | ||
objectMetadataItems, | ||
shouldMatchRootQueryFilter, | ||
}); | ||
|
||
setLoading(false); | ||
}, | ||
}) | ||
.catch((error: Error) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider wrapping the error handling logic in a separate function to improve readability and reusability across similar hooks. |
||
if (!recordCreatedInCache) { | ||
throw error; | ||
} | ||
|
||
deleteRecordFromCache({ | ||
objectMetadataItems, | ||
objectMetadataItem, | ||
cache: apolloClient.cache, | ||
recordToDelete: recordCreatedInCache, | ||
}); | ||
|
||
triggerCreateRecordsOptimisticEffect({ | ||
cache, | ||
triggerDeleteRecordsOptimisticEffect({ | ||
cache: apolloClient.cache, | ||
objectMetadataItem, | ||
recordsToCreate: [record], | ||
recordsToDelete: [recordCreatedInCache], | ||
objectMetadataItems, | ||
shouldMatchRootQueryFilter, | ||
}); | ||
|
||
setLoading(false); | ||
}, | ||
}); | ||
throw error; | ||
}); | ||
|
||
return createdObject.data?.[mutationResponseField] ?? null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: This line might throw an error if createdObject is undefined due to a caught exception. Consider adding a null check. |
||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
import { useApolloClient } from '@apollo/client'; | ||
|
||
import { triggerCreateRecordsOptimisticEffect } from '@/apollo/optimistic-effect/utils/triggerCreateRecordsOptimisticEffect'; | ||
import { triggerDeleteRecordsOptimisticEffect } from '@/apollo/optimistic-effect/utils/triggerDeleteRecordsOptimisticEffect'; | ||
import { apiConfigState } from '@/client-config/states/apiConfigState'; | ||
import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; | ||
import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; | ||
import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; | ||
import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache'; | ||
import { DEFAULT_MUTATION_BATCH_SIZE } from '@/object-record/constants/DefaultMutationBatchSize'; | ||
import { useDeleteManyRecordsMutation } from '@/object-record/hooks/useDeleteManyRecordsMutation'; | ||
import { getDeleteManyRecordsMutationResponseField } from '@/object-record/utils/getDeleteManyRecordsMutationResponseField'; | ||
|
@@ -65,38 +67,74 @@ export const useDeleteManyRecords = ({ | |
(batchIndex + 1) * mutationPageSize, | ||
); | ||
|
||
const deletedRecordsResponse = await apolloClient.mutate({ | ||
mutation: deleteManyRecordsMutation, | ||
variables: { | ||
filter: { id: { in: batchIds } }, | ||
}, | ||
optimisticResponse: options?.skipOptimisticEffect | ||
? undefined | ||
: { | ||
[mutationResponseField]: batchIds.map((idToDelete) => ({ | ||
__typename: capitalize(objectNameSingular), | ||
id: idToDelete, | ||
const deletedRecordsResponse = await apolloClient | ||
.mutate({ | ||
mutation: deleteManyRecordsMutation, | ||
variables: { | ||
filter: { id: { in: batchIds } }, | ||
}, | ||
optimisticResponse: options?.skipOptimisticEffect | ||
? undefined | ||
: { | ||
[mutationResponseField]: batchIds.map((idToDelete) => ({ | ||
__typename: capitalize(objectNameSingular), | ||
id: idToDelete, | ||
})), | ||
}, | ||
update: options?.skipOptimisticEffect | ||
? undefined | ||
: (cache, { data }) => { | ||
const records = data?.[mutationResponseField]; | ||
|
||
if (!records?.length) return; | ||
|
||
const cachedRecords = records | ||
.map((record) => getRecordFromCache(record.id, cache)) | ||
.filter(isDefined); | ||
|
||
triggerDeleteRecordsOptimisticEffect({ | ||
cache, | ||
objectMetadataItem, | ||
recordsToDelete: cachedRecords, | ||
objectMetadataItems, | ||
}); | ||
}, | ||
}) | ||
.catch((error: Error) => { | ||
const cachedRecords = batchIds.map((idToDelete) => | ||
getRecordFromCache(idToDelete, apolloClient.cache), | ||
); | ||
|
||
cachedRecords.forEach((cachedRecord) => { | ||
if (!cachedRecord) { | ||
return; | ||
} | ||
|
||
updateRecordFromCache({ | ||
objectMetadataItems, | ||
objectMetadataItem, | ||
cache: apolloClient.cache, | ||
record: { | ||
...cachedRecord, | ||
deletedAt: null, | ||
}, | ||
}); | ||
}); | ||
Comment on lines
+108
to
+122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider wrapping this forEach loop in a try-catch block to handle potential errors during the revert process. |
||
|
||
triggerCreateRecordsOptimisticEffect({ | ||
cache: apolloClient.cache, | ||
objectMetadataItem, | ||
objectMetadataItems, | ||
recordsToCreate: cachedRecords | ||
.filter(isDefined) | ||
.map((cachedRecord) => ({ | ||
...cachedRecord, | ||
deletedAt: null, | ||
})), | ||
}, | ||
update: options?.skipOptimisticEffect | ||
? undefined | ||
: (cache, { data }) => { | ||
const records = data?.[mutationResponseField]; | ||
|
||
if (!records?.length) return; | ||
|
||
const cachedRecords = records | ||
.map((record) => getRecordFromCache(record.id, cache)) | ||
.filter(isDefined); | ||
|
||
triggerDeleteRecordsOptimisticEffect({ | ||
cache, | ||
objectMetadataItem, | ||
recordsToDelete: cachedRecords, | ||
objectMetadataItems, | ||
}); | ||
}, | ||
}); | ||
}); | ||
|
||
throw error; | ||
}); | ||
|
||
const deletedRecordsForThisBatch = | ||
deletedRecordsResponse.data?.[mutationResponseField] ?? []; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: setLoading(false) should also be called in the catch block to ensure the loading state is reset on error.