Closed kolodi closed 8 months ago
svidgen
commented
1 year ago Hey, it looks like you're trying to save updates on top of a record returned from DataStore.save(). Our guidance is to always re-query for the record using DataStore.query() before updating. Can you give that a try and let us know if it works?
kolodi
commented
1 year ago Hey, it looks like you're trying to save updates on top of a record returned from
DataStore.save(). Our guidance is to always re-query for the record usingDataStore.query()before updating. Can you give that a try and let us know if it works?
Yes, I've tried to do requery the item after update by triggering useEffect with a progressive version. It still was failing, until I have added 1 second delay after saving before refetching again. I wonder of there is a better way to know when i must refetch.
const [editingVersion, setVersion] = useState<number>(0);
async function saveChanges() {
const original = item!;
console.log("save", original, changes);
const n = await DataStore.save<T>(itemType.copyOf(original, updated => {
Object.assign(updated, changes);
}));
// this helped
await new Promise((resolve) => { setTimeout(resolve, 1000) });
console.log("saved", n);
setEditingVersion(v => v + 1); // this will trigger useEffect to refetch item
}
useEffect(() => {
setIsLoading(true);
console.log("fetching item", itemId, itemType.name);
fetchItem<T>({ itemType, itemId }).then((item) => {
if (isMounted()) {
console.log("fetched item", item);
setItem(item);
setChanges({});
}
}).catch((error) => {
if (isMounted()) {
console.log(error);
setError(error);
}
}).finally(() => {
if (isMounted()) {
setIsLoading(false);
}
});
}, [itemType, isMounted, itemId, editingVersion]);
cwomack
commented
1 year ago @kolodi, the delay that was introduced may have helped with the time it takes for the mutation to go out (depending on connection speed). If you're looking to be more deterministic for this, you could watch for Hub events like outboxMutationProcessed and introduce more controls over editing and saving data. You could also look for specific models being sent or wait for outbox to be empty (via outboxStatus event) to ensure everything is synched before you allow more edits.
Let me know if that helps you accomplish a better way to know when you need to refetch!
kolodi
commented
1 year ago Hey @cwomack, It seems like the best pattern so far is to use subscription. Initially I thought I might not need them. I have been removing subscriptions from GraphQL schema. But when working with DataStore it is basically assumed that you use them. It stamps warnings when you miss them. But it is not written anywhere in the docs. There should be an official guidelines for the DataStore and the usage of subscription pattern I think, as it is the way to go imo.
```typescript
import { DataStore } from "aws-amplify";
import { PersistentModelConstructor, PersistentModel, IdentifierFieldOrIdentifierObject, PersistentModelMetaData } from "@aws-amplify/datastore";
import { useEffect, useState } from "react";
import deepEqual from "deep-equal";
import { useAbortablePromise } from "../../helpers/useAbortablePromise";
export type UseItemData
useAbortablePromise - is just to avoid fetching during multiple remounting during development.
Still this method has issues. Mainly because the observe will trigger at least 3 times when online. And there is no errors coming when the sync with the cloud is failed.
I guess, there is a way to do it by subscribing to the Hub sync messages. And it would be even more complex. Because on one hand you need an updated item and then you need to intercept eventual sync errors, compare if it is related to the item being edited, probably restore the previous item data (as the edit failed, you want the changes to be rolled back and not use locally stored wrong data that can't be synced), and display some meaningful message to the user about the error.
It would be nice to have all the necessary info coming from observe->subscribe message. Besides opType, having things like: sync completed or sync failed, offline/online mode, when sync error occurred provide previous model state. So that you can just update your editing item immediately when in offline, or wait until sync is completed and then update it only once or roll back and display error message to the user.
Hub events then still can be used for the complex offline scenarios, when you need to display sync errors later for the items that are not synced properly once you are back online.
One more thing: It also would be nice to have initial item coming from subscribe. Like first time you subscribe you get back immediately an item even if it is not yet changed, just to avoid additional setup for the first fetch. Like in my code example, I need to fetch the item the first time when mounting the component. But if after subscribing to the observe by id would give you an item at least once, I could just use subscribe and no initial fetch. It is possible when working with lists tho, like so:
```typescript
useEffect(() => {
// criteria is for paginating / filtering
const o = DataStore.observeQuery
Here I can rely on having the initial list coming from the observe->subscribe rather than fetching it on my own.
kolodi
commented
8 months ago Some final thoughts and useful code snippets
DataStore.queryObserve is not ideal when rendering big data sets, mainly because of 2 reasons:
DataStore.queryIf you need a proper pagination I generally use query and manually triggering "refetch" when needed:
import { useQuery } from "@tanstack/react-query";
import { DataStore, PersistentModel, PersistentModelConstructor, ProducerPaginationInput, RecursiveModelPredicateExtender } from "aws-amplify/datastore";
import { useState } from "react";
type ObserveQueryProps<T extends PersistentModel> = {
modelConstructor: PersistentModelConstructor<T>
criteria?: RecursiveModelPredicateExtender<T>
paginationProducer?: ProducerPaginationInput<T>
criteriaDeps?: any[]
}
/**
* Query items list from DataStore with working pagination.
* Use triggerQuery function to manually trigger refetch.
* Criteria is a function and it cannot be used as a dependance to trigger refetch,
* therefore use a separate criteriaDeps, most often the same deps used to recreate criteria
*/
export default function useListQuery<T extends PersistentModel>({
modelConstructor,
criteria,
paginationProducer,
criteriaDeps = [],
}: ObserveQueryProps<T>) {
const [v, setV] = useState(0)
function triggerQuery() {
setV(p => p + 1)
}
const { data: items, isLoading, error } = useQuery({
queryKey: [modelConstructor, paginationProducer, updateVersion, ...criteriaDeps],
queryFn: async () => DataStore.query(modelConstructor, criteria, paginationProducer),
initialData: [],
})
return {
items,
isLoading,
error,
triggerQuery,
}
}When working with small lists (can be also filtered with criteria that does not change very often), and when you don't care about pagination and rerenderings, you can DataStore.observeQuery:
import { DataStore, PersistentModel, PersistentModelConstructor, ProducerPaginationInput, RecursiveModelPredicateExtender } from "aws-amplify/datastore";
import { useEffect, useState } from "react";
type ObserveQueryProps<T extends PersistentModel> = {
modelConstructor: PersistentModelConstructor<T>
criteria?: RecursiveModelPredicateExtender<T>
paginationProducer?: ProducerPaginationInput<T>
}
/**
* Observed query for list items. Note: Pagination is not working properly here. For pagination use useQueryList
*/
export default function useDataStoreListObserved<T extends PersistentModel>({
modelConstructor,
criteria,
paginationProducer,
}: ObserveQueryProps<T>) {
const [items, setItems] = useState<T[]>([])
useEffect(() => {
const sub = DataStore
.observeQuery(modelConstructor, criteria, paginationProducer)
.subscribe(({ items }) => {
setItems(items)
})
return () => sub.unsubscribe()
}, [modelConstructor, criteria, paginationProducer])
return items
}For a single item I usually combine 2 approaches:
import { useEffect, useState } from "react"
import { useQuery } from "@tanstack/react-query"
import {
DataStore,
IdentifierFieldOrIdentifierObject,
PersistentModel,
PersistentModelConstructor,
PersistentModelMetaData,
} from "aws-amplify/datastore"
export default function useDataStoreItemObserved<T extends PersistentModel>(
itemType: PersistentModelConstructor<T>,
id: IdentifierFieldOrIdentifierObject<T, PersistentModelMetaData<T>>,
) {
const [v, setV] = useState(0)
const query = useQuery({
queryKey: [itemType, id, v],
queryFn: () => DataStore.query(itemType, id),
})
useEffect(() => {
const sub = DataStore
.observe(itemType, id as string)
.subscribe(() => {
setV(vv => vv + 1)
})
return () => sub.unsubscribe()
}, [itemType, id])
return query
}Note useQuery here is just to solve annoying async state management issues in react and not for actual network requests.
This issue can be closed i suppose.
The problem that persists for me is that there is no elegant way to avoid unnecessary rerenderings when getting updates in observe and observeQuery. In offline mode you get 1 update, when online you get at least 3. Ideally I'd like to have some sort of flag indicating "final" update for any given operation. For example, when updating an item, if you are in offline mode, you will get am update event with flag "final" set to true soon after local successful update (or error in case there is one), when online, the successful local update should give "final" false, only after successful remote sync you should see the flag to be true. Or, alternately, an "update" status property to know: saved locally but yet to be synced to remote (wait), saved locally but remote sync unavailable (rerender), saved remotely (rerender), error saving locally or remotely (display error). Yes , I really want save/sync errors to be obtained in subscription payload.
chrisbonifacio
commented
8 months ago @kolodi I'll go ahead and close this out. I do want to thank you for taking the time to leave feedback and your findings! We'll take this into consideration for improvements to be made.
If you run into any other issues, please feel free to open a new issue.
chrisbonifacio
commented
8 months ago Also, surfacing save/sync errors through subscriptions would definitely be a feature request. I'm not sure how we could do that however. As far as I understand, the subscriptions are triggered by successful mutations via websocket connection from AppSync rather than our library but perhaps they can be surfaced through the DataStore events instead.
Please also feel free to open a feature request with the details
david-mcafee
commented
7 months ago @kolodi - In response to your concern around re-renders, there is an option to monitor the isSynced status flag to avoid unnecessary re-renders.
Additionally, you are correct that DataStore will return three subscription messages per mutation when online. If you're concerned about the re-renders from those additional messages, you could also add a check to see if the data has changed before processing the payload.
Lastly, you mentioned that it would be helpful to know when a sync has completed, when DataStore is offline/online, etc. I wanted to link to the DataStore events docs if you weren't already aware of them, as DataStore does publish messages for these events.
Pagination is not working as you expect, it basically ignores small limit parameter so it is very different from results obtained from DataStore.query
Are you referring here to adjusting syncPageSize when calling DataStore.configure? I apologize, but I wasn't sure what you meant by it ignoring small limit parameters. If there is any additional guidance we can provide here, please let us know!
Thanks!
Before opening, please confirm:
JavaScript Framework
Next.js
Amplify APIs
DataStore
Amplify Categories
No response
Environment information
Describe the bug
I have an edit form to make changes to a model. First
DataStore.query()is used to get the Model data. Here is an example code of saving changes:It works fine the first time. The second time I try to save new changes, the
DataStore.save()itself runs well, but then there is an error during synchronization to the backend. It complains that there are required data missing. I have debugged the sync process and basically, it tries to update the Asset model with onlyItemNameproperty in it and there are no other previously existed properties. I have debugged again theDataStore.save() function to see that there are all the necessary properties, thusoriginalstill contains all the data. And I think theDataStore.save()would have thrown an error in this case even before the sync process. But it throws nothing. TheupdatedModelis returned correctly.Expected behavior
The model should be updated many times without errors not only the first time.
Reproduction steps
Code Snippet
Log output
aws-exports.js
No response
Manual configuration
No response
Additional configuration
No response
Mobile Device
No response
Mobile Operating System
No response
Mobile Browser
No response
Mobile Browser Version
No response
Additional information and screenshots
No response