Skip to content
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

Merged
merged 6 commits into from
Sep 9, 2024

Conversation

Ldoppea
Copy link
Member

@Ldoppea Ldoppea commented May 23, 2024

We want to reduce coupling between CozyPouchLink and the browser's
local storage

This PR adds a platform option into the PouchLink constructor in
order 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 and document.removeEventListener API are
available

If no custom platform is injected, then platformWeb.js will be used
by default

In order to inject a custom platform, create a new platform object
containing the same API as platformWeb.js and add it into the
PouchLink constructor's platform option

import { default as PouchLink } from 'cozy-pouch-link'

// Class based on `platformWeb.js`
import { CustomPlaftorm } from './CustomPlaftorm'

const pouchLink = new PouchLink({
  platform: new CustomPlaftorm()
})

Related PRs:

export class PouchLocalStorage {
constructor(storageEngine) {
this.storageEngine = storageEngine
}
Copy link
Contributor

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 ?

Copy link
Member Author

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
Copy link
Contributor

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)
Copy link
Contributor

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})

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #1524

@Ldoppea Ldoppea force-pushed the feat/pouch_react_native branch 7 times, most recently from c376d72 to 255e57e Compare July 17, 2024 07:57
@Ldoppea Ldoppea changed the base branch from master to feat/enable_type_generation_pouch July 26, 2024 14:26
Base automatically changed from feat/enable_type_generation_pouch to feat/meta_offline July 26, 2024 14:41
@Ldoppea Ldoppea marked this pull request as ready for review July 26, 2024 14:46
@Ldoppea Ldoppea requested a review from Merkur39 as a code owner July 26, 2024 14:46
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
Copy link
Contributor

@Crash-- Crash-- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Copy link
Contributor

@paultranvan paultranvan left a 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!

Ldoppea added a commit that referenced this pull request Sep 4, 2024
In order to prevent implementation errors, we want to check that
storageEngine implements the correct methods

This replies to #1483 (comment)
Ldoppea added a commit that referenced this pull request Sep 4, 2024
@Ldoppea Ldoppea merged commit ac26a42 into feat/meta_offline Sep 9, 2024
2 checks passed
@Ldoppea Ldoppea deleted the feat/pouch_react_native branch September 9, 2024 09:59
Ldoppea added a commit that referenced this pull request Sep 9, 2024
In order to prevent implementation errors, we want to check that
storageEngine implements the correct methods

This replies to #1483 (comment)
Ldoppea added a commit that referenced this pull request Sep 9, 2024
Ldoppea added a commit that referenced this pull request Sep 9, 2024
In order to prevent implementation errors, we want to check that
storageEngine implements the correct methods

This replies to #1483 (comment)
Ldoppea added a commit that referenced this pull request Sep 9, 2024
Ldoppea added a commit that referenced this pull request Sep 9, 2024
In order to prevent implementation errors, we want to check that
storageEngine implements the correct methods

This replies to #1483 (comment)
Ldoppea added a commit that referenced this pull request Sep 9, 2024
Ldoppea added a commit that referenced this pull request Sep 19, 2024
In order to prevent implementation errors, we want to check that
storageEngine implements the correct methods

This replies to #1483 (comment)
Ldoppea added a commit that referenced this pull request Sep 19, 2024
Ldoppea added a commit that referenced this pull request Sep 19, 2024
In order to prevent implementation errors, we want to check that
storageEngine implements the correct methods

This replies to #1483 (comment)
Ldoppea added a commit that referenced this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants