-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: Make CozyPouchLink compatible with ReactNative #1483
Conversation
6fe85a7
to
cb0062d
Compare
ca6071c
to
bdea993
Compare
export class PouchLocalStorage { | ||
constructor(storageEngine) { | ||
this.storageEngine = storageEngine | ||
} |
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.
Maye it would be worth checking the expected methods getItem
, setItem
, removeItem
are provided by the storage engine ?
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.
Done in #1524
) | ||
this.PouchDB = options.platform?.pouchAdapter || platformWeb.pouchAdapter | ||
this.isOnline = options.platform?.isOnline || platformWeb.isOnline | ||
this.events = options.platform?.events || platformWeb.events |
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.
I wonder if we should enforce some strict checking on the given options.platform
as it has a lot of responsibility and it's easy to miss something
@@ -68,7 +67,7 @@ export const startReplication = ( | |||
// For the first remote->local replication, we manually replicate all docs | |||
// as it avoids to replicate all revs history, which can lead to | |||
// performances issues | |||
docs = await replicateAllDocs(pouch, url, doctype) | |||
docs = await replicateAllDocs(pouch, url, doctype, storage) |
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.
nit: I might be worth moving to await replicateAllDocs({pouch, url, doctype, storage})
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.
Done in #1524
c376d72
to
255e57e
Compare
16237f8
to
0049a23
Compare
We want to reduce coupling between CozyPouchLink and the browser's local storage The first step is to groupe existing methods into a PouchLocalStorage class, this will allow (in the next commit) to inject a storage adapter into the class constructor Also in some platforms like ReactNative, the localstorage methods are asynchronous. So in order to be compatible with all platforms we want those methods to become async by default The big main of making those methods async is that we cannot call them from the `PouchManager` constructor. So we introduce a new `.init()` method in the `PouchManager` class. This async method now contains all the initialization logic and should be called after creating a new `PouchManager` As `PouchManager` is called internally by `CozyPouchLink` and is not meant to be a publicly available class, then we don't consider this as a breaking change
We want to reduce coupling between CozyPouchLink and the browser's local storage This will allow to use CozyPouchLink from a react-native project where `window.localStorage` API is not available If no custom storage is injected, then `platformWeb.js` will be used by default In order to inject a custom storage, create a new platform object containing the same API as `platformWeb.js` and add it into the PouchLink constructor's `platform` option ```js import { default as PouchLink } from 'cozy-pouch-link' // Class based on `platformWeb.js` import { CustomPlaftorm } from './CustomPlaftorm' const pouchLink = new PouchLink({ platform: new CustomPlaftorm() }) ```
In previous commit we added a `platform` option into the PouchLink constructor in order to allow injecting a custom local storage API from a react-native project We also want to inject a custom PouchDB adapter as a react-native project would use a different PouchDB implementation Like for the local storage API, if no injection is given, then `pouchdb-browser` adapter will be used by default
In previous commit we added a `platform` option into the PouchLink constructor in order to allow injecting custom local storage API and PouchDB adapter from a react-native project We also want to inject a custom `isOnline` method as react-native does not provide the `window.navigator.onLine` API Like for the other APIs, if no injection is given, then `window.navigator.onLine` will be used by default
In previous commit we added a `platform` option into the PouchLink constructor in order to allow injecting custom local storage API, PouchDB adapter and isOnline method from a react-native project We also want to inject a custom evant emitter for online/offline and pause/resume events as react-native does not provide the `document.addEventListener` and `document.removeEventListener` APIs Like for the other APIs, if no injection is given, then `document` APIs will be used by default
255e57e
to
0037e6f
Compare
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.
Nice work!
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.
Excellent work, with very good atomic commits and descriptions!
In order to prevent implementation errors, we want to check that storageEngine implements the correct methods This replies to #1483 (comment)
In order to prevent implementation errors, we want to check that storageEngine implements the correct methods This replies to #1483 (comment)
In order to prevent implementation errors, we want to check that storageEngine implements the correct methods This replies to #1483 (comment)
In order to prevent implementation errors, we want to check that storageEngine implements the correct methods This replies to #1483 (comment)
In order to prevent implementation errors, we want to check that storageEngine implements the correct methods This replies to #1483 (comment)
In order to prevent implementation errors, we want to check that storageEngine implements the correct methods This replies to #1483 (comment)
In order to prevent implementation errors, we want to check that storageEngine implements the correct methods This replies to #1483 (comment)
In order to prevent implementation errors, we want to check that storageEngine implements the correct methods This replies to #1483 (comment)
We want to reduce coupling between CozyPouchLink and the browser's
local storage
This PR adds a
platform
option into the PouchLink constructor inorder to allow injecting custom local storage API, PouchDB adapter, an
isOnline method and a custom event emitter for online/offline and
pause/resume events from the consuming app
This will allow to use CozyPouchLink from a react-native project where
a different PouchDB adapter is used and where none of
window.localStorage
,window.navigator.onLine
,document.addEventListener
anddocument.removeEventListener
API areavailable
If no custom
platform
is injected, thenplatformWeb.js
will be usedby default
In order to inject a custom
platform
, create a new platform objectcontaining the same API as
platformWeb.js
and add it into thePouchLink constructor's
platform
optionRelated PRs: