Add support for offline/local first applications #10545
Conversation
| EditPostPage.setInputValue( | ||
| 'input', | ||
| 'title', | ||
| 'Lorem Ipsum again{enter}' |
There was a problem hiding this comment.
Because react-query now persist queries and mutations for offline mode, the previous test now leaks into the second (e.g. this post has its title changed to Lorem Ipsum). I tried to configure testIsolation in Cypress but our version is probably too old
djhi
force-pushed
the
support-offline-mode
branch
from
d440e8e to
9d66e05
Compare
|
I can't see a way to test this except e2e. Not sure if we can simulate network loss with cypress though |
|
@djhi since the simple demo has the react-query DevTools enabled, you can use them to simulate network loss I believe (clicking on the 'Wifi' icon does the job). A bit ugly but it should work! |
packages/ra-core/src/controller/input/useReferenceInputController.ts
Outdated
Show resolved
Hide resolved
examples/simple/src/Layout.tsx
Outdated
| const queryClient = useQueryClient(); | ||
| const notify = useNotify(); | ||
|
|
||
| queryClient.setMutationDefaults([], { |
There was a problem hiding this comment.
this looks like an effect, why do you execute it on render?
examples/simple/src/Layout.tsx
Outdated
| queryClient.setMutationDefaults([], { | ||
| onSettled(data, error) { | ||
| if (error) { | ||
| notify(error.message, { type: 'error' }); |
There was a problem hiding this comment.
This will result in a doubled mutation with e.g. <Edit>, which already shows an error notification with onError. Also, I don't understand the point of showing directly the error message...
There was a problem hiding this comment.
Did you actually try it? This effect only runs when there are persisted mutations to resume. For instance, create a new post with title f00bar in optimistic mode while offline and click Save and edit then close your browser tab. Reopen the tab, restore the network if needed. You'll see the server error as expected.
However, should you do the same while offline, the above function will not be called
examples/simple/src/Layout.tsx
Outdated
| </> | ||
| ); | ||
| export default ({ children }) => { | ||
| return ( |
There was a problem hiding this comment.
please keep the simple functional syntax for brevity.
examples/simple/src/Layout.tsx
Outdated
| setIsOnline(isOnline); | ||
| }; | ||
| return onlineManager.subscribe(handleChange); | ||
| }); |
There was a problem hiding this comment.
Why do we need to run this effect every time the app bar renders (which is often) instead of just on mount?
| > | ||
| <Alert severity="info" role="presentation"> | ||
| To test offline support, add either{' '} | ||
| <Link to="/posts/create?mutationMode=optimistic"> |
There was a problem hiding this comment.
Why don't you use a special location state instead?
There was a problem hiding this comment.
I don't get this comment
| * </PersistQueryClientProvider> | ||
| * ); | ||
| */ | ||
| export const addOfflineSupportToQueryClient = ({ |
There was a problem hiding this comment.
As explained earlier, can't we create a new Query Client instead? or at least return the modified one.
| @@ -8,6 +8,14 @@ export const UPDATE_MANY = 'UPDATE_MANY'; | |||
| export const DELETE = 'DELETE'; | |||
| export const DELETE_MANY = 'DELETE_MANY'; | |||
|
|
|||
| export const reactAdminMutations = [ | |||
There was a problem hiding this comment.
"data provider mutations" would be a better name IMHO
| @@ -176,7 +176,7 @@ export const useGetList = < | |||
| } | |||
| : result, | |||
| [result] | |||
| ) as UseQueryResult<RecordType[], Error> & { | |||
| ) as unknown as UseQueryResult<RecordType[], Error> & { | |||
| !Object.keys(filterValues).length && | ||
| // there is an empty page component | ||
| empty !== false; | ||
| (isPaused && isPlaceholderData) || |
There was a problem hiding this comment.
I don't understand the rationale of showing the empty page when offline. This gives a wrong information.
There was a problem hiding this comment.
Should we introduce a new offline prop and <Offline> component?
Co-authored-by: Francois Zaninotto <[email protected]>
That's the only one left AFAIK |
Problem
React-admin currently don't handle well the loss of network connectivity: in any demo, if you go to a list page, then simulate being offline using the devtool and try to go to the second page of the same list, it will still display the first page data while the pagination will indeed indicate you are on page 2.
We handled it a bit better by at least showing users a notification when they try to load new data while offline but we can do better and allow for resumable mutations.
Solution
ListContextinclude theisPausedandisPlaceholderDataprops from react-query and update components/hooks accordinglyemptycomponent when those two props are true (ideally we should have a dedicated component for that or handle the case in the default empty but this is achievable in userland)mutationKeyto all mutations so that we can provide default functions for them at thequeryClientlevel (required by react-query: https://tanstack.com/query/latest/docs/framework/react/guides/mutations#persisting-offline-mutations)offlineprop to all components fetching dataHow To Test
Then:
?mutationMode=optimisticlink first.?mutationMode=undoablelink first.f00baras the post title to trigger an error once the network is online.Additional Checks
masterfor a bugfix, ornextfor a featureAlso, please make sure to read the contributing guidelines.