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

[Fix] Common SDK fixes for Web and React Native #24

Merged
merged 22 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/beige-vans-hunt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@journeyapps/powersync-sdk-react-native': patch
---

Fixed: `get`, `getAll` and `getOptional` should execute inside a readLock for concurrency
6 changes: 6 additions & 0 deletions .changeset/five-turtles-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@journeyapps/powersync-sdk-common': patch
---

- Removed `user-id` header from backend connector and remote headers
- Mark PowerSync client as pending initialization before `init` has been called.
5 changes: 5 additions & 0 deletions .changeset/rare-zoos-walk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@journeyapps/powersync-react': patch
---

Fixed: Added correct typings for React hooks. Previously hooks would return `any`.
2 changes: 1 addition & 1 deletion apps/supabase-todolist
7 changes: 5 additions & 2 deletions packages/powersync-attachments/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@
"dependencies": {
"@journeyapps/powersync-sdk-common": "0.1.1"
},
"repository": "https://github.com/journeyapps/powersync-react-native-sdk",
"repository": {
"type": "git",
"url": "git+https://github.com/powersync-ja/powersync-react-native-sdk.git"
},
"author": "JOURNEYAPPS",
"license": "Apache-2.0",
"homepage": "https://docs.powersync.co/resources/api-reference",
"bugs": {
"url": "https://github.com/journeyapps/powersync-react-native-sdk/issues"
"url": "https://github.com/powersync-ja/powersync-react-native-sdk/issues"
}
}
12 changes: 8 additions & 4 deletions packages/powersync-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,25 @@
"clean": "rm -rf lib tsconfig.tsbuildinfo",
"watch": "tsc -b -w"
},
"repository": "https://github.com/journeyapps/powersync-react-native-sdk",
"repository": {
"type": "git",
"url": "git+https://github.com/powersync-ja/powersync-react-native-sdk.git"
},
"author": "JOURNEYAPPS",
"license": "Apache-2.0",
"bugs": {
"url": "https://github.com/journeyapps/powersync-react-native-sdk/issues"
"url": "https://github.com/powersync-ja/powersync-react-native-sdk/issues"
},
"homepage": "https://docs.powersync.co/resources/api-reference",
"dependencies": {
"@journeyapps/powersync-sdk-common": "^0.1.0"
"@journeyapps/powersync-sdk-common": "0.1.1"
},
"peerDependencies": {
"react": "*"
},
"devDependencies": {
"@types/react": "^18.2.34",
"react": "18.2.0",
"typescript": "^4.1.3"
"typescript": "^5.1.3"
}
}
9 changes: 6 additions & 3 deletions packages/powersync-sdk-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@
"files": [
"lib"
],
"repository": "https://github.com/journeyapps/powersync-react-native-sdk",
"repository": {
"type": "git",
"url": "git+https://github.com/powersync-ja/powersync-react-native-sdk.git"
},
"bugs": {
"url": "https://github.com/journeyapps/powersync-react-native-sdk/issues"
"url": "https://github.com/powersync-ja/powersync-react-native-sdk/issues"
},
"homepage": "https://docs.powersync.co/resources/api-reference",
"scripts": {
Expand All @@ -27,7 +30,7 @@
"@types/node": "^20.5.9",
"@types/object-hash": "^3.0.4",
"@types/uuid": "^3.0.0",
"typescript": "^4.1.3"
"typescript": "^5.1.3"
},
"dependencies": {
"async-mutex": "^0.4.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,28 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver<PowerSyncDB
private syncStatusListenerDisposer?: () => void;
protected initialized: Promise<void>;

/**
* Initializes PowerSync
*/
public init: () => Promise<void>;

constructor(protected options: PowerSyncDatabaseOptions) {
super();
this.currentStatus = null;
this.closed = true;
this.options = { ...DEFAULT_POWERSYNC_DB_OPTIONS, ...options };
this.bucketStorageAdapter = this.generateBucketStorageAdapter();
this.sdkVersion = '';
// This ensures the `initialized` promise is pending even before `init` is called
this.initialized = new Promise((resolve, reject) => {
this.init = async () => {
try {
resolve(await this.initFn());
} catch (ex) {
reject(ex);
}
};
});
Manrich121 marked this conversation as resolved.
Show resolved Hide resolved
}

get schema() {
Expand All @@ -99,15 +114,16 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver<PowerSyncDB
protected abstract generateBucketStorageAdapter(): BucketStorageAdapter;

abstract _init(): Promise<void>;
async init() {
this.initialized = (async () => {
await this._init();
await this.bucketStorageAdapter.init();
await this.database.execute('SELECT powersync_replace_schema(?)', [JSON.stringify(this.schema.toJSON())]);
const version = await this.options.database.execute('SELECT powersync_rs_version()');
this.sdkVersion = version.rows?.item(0)['powersync_rs_version()'] ?? '';
})();
await this.initialized;

/**
* This performs the total initialization process.
*/
private async initFn() {
await this._init();
await this.bucketStorageAdapter.init();
await this.database.execute('SELECT powersync_replace_schema(?)', [JSON.stringify(this.schema.toJSON())]);
const version = await this.options.database.execute('SELECT powersync_rs_version()');
this.sdkVersion = version.rows?.item(0)['powersync_rs_version()'] ?? '';
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export interface PowerSyncCredentials {
endpoint: string;
token: string;
userID?: string;
expiresAt?: Date;
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,6 @@ export class SqliteBucketStorage implements BucketStorageAdapter {
*/
private async updateObjectsFromBuckets(checkpoint: Checkpoint) {
return this.writeTransaction(async (tx) => {
/**
* It's best to execute this on the same thread
* https://github.com/journeyapps/powersync-sqlite-core/blob/40554dc0e71864fe74a0cb00f1e8ca4e328ff411/crates/sqlite/sqlite/sqlite3.h#L2578
*/
const { insertId: result } = await tx.execute('INSERT INTO powersync_operations(op, data) VALUES(?, ?)', [
'sync_local',
''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export abstract class AbstractRemote {
const credentials = await this.getCredentials();
return {
'content-type': 'application/json',
'User-Id': credentials.userID ?? '',
Authorization: `Token ${credentials.token}}`
};
}
Expand Down
10 changes: 9 additions & 1 deletion packages/powersync-sdk-react-native/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# PowerSync SDK for React Native

[PowerSync](https://powersync.co) is a service and set of SDKs that keeps Postgres databases in sync with on-device SQLite databases. See a summary of features [here](https://docs.powersync.co/resources/api-reference#react-native-and-expo).
[PowerSync](https://powersync.co) is a service and set of SDKs that keeps Postgres databases in sync with on-device SQLite databases. See a summary of features [here](https://docs.powersync.co/client-sdk-references/react-native-and-expo).

## Beta Release
This React Native SDK package is currently in a beta release.
Expand Down Expand Up @@ -178,5 +178,13 @@ Uncomment the following from
// });
// client.addPlugin(networkFlipperPlugin);
```

Disable the dev client network inspector
`android/gradle.properties`
```
# Enable network inspector
EX_DEV_CLIENT_NETWORK_INSPECTOR=false
```

## iOS
Testing offline mode on an iOS simulator by disabling the host machine's entire internet connection will cause the device to remain offline even after the internet connection has been restored. This issue seems to affect all network requests in an application.
13 changes: 8 additions & 5 deletions packages/powersync-sdk-react-native/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,18 @@
"clean": "rm -rf lib tsconfig.tsbuildinfo",
"watch": "tsc -b -w"
},
"repository": "https://github.com/journeyapps/powersync-react-native-sdk",
"repository": {
"type": "git",
"url": "git+https://github.com/powersync-ja/powersync-react-native-sdk.git"
},
"author": "JOURNEYAPPS",
"license": "Apache-2.0",
"bugs": {
"url": "https://github.com/journeyapps/powersync-react-native-sdk/issues"
"url": "https://github.com/powersync-ja/powersync-react-native-sdk/issues"
},
"homepage": "https://docs.powersync.co/",
"peerDependencies": {
"@journeyapps/react-native-quick-sqlite": "0.1.0",
"@journeyapps/react-native-quick-sqlite": "0.0.0-dev-20231103124824",
"base-64": "^1.0.0",
"react": "*",
"react-native": "*",
Expand All @@ -41,11 +44,11 @@
"async-lock": "^1.4.0"
},
"devDependencies": {
"@journeyapps/react-native-quick-sqlite": "0.1.0",
"@journeyapps/react-native-quick-sqlite": "0.0.0-dev-20231103124824",
"@types/async-lock": "^1.4.0",
"react-native": "0.72.4",
"react": "18.2.0",
"typescript": "^4.1.3"
"typescript": "^5.1.3"
},
"keywords": [
"data sync",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ export class RNQSDBAdapter extends BaseObserver<DBAdapterListener> implements DB
this.iterateListeners((cb) => cb.tablesUpdated?.(update));
});

const topLevelUtils = this.generateDBHelpers({ execute: this.baseDB.execute });
const topLevelUtils = this.generateDBHelpers({
// Arrow function binds `this` for use in readOnlyExecute
execute: (sql: string, params?: any[]) => this.readOnlyExecute(sql, params)
});
// Only assigning get helpers
this.getAll = topLevelUtils.getAll;
this.getOptional = topLevelUtils.getOptional;
this.get = topLevelUtils.get;
Expand Down Expand Up @@ -55,6 +59,16 @@ export class RNQSDBAdapter extends BaseObserver<DBAdapterListener> implements DB
return this.baseDB.execute(query, params);
}

/**
* This provides a top-level read only execute method which is executed inside a read-lock.
* This is necessary since the high level `execute` method uses a write-lock under
* the hood. Helper methods such as `get`, `getAll` and `getOptional` are read only,
* and should use this method.
Comment on lines +63 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a top-level read-only execute method?
According to the comment, get, getAll and getOptional should use this, but it does not seem to be the case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The DB adapter and the powersync client expose SQL execution methods such as execute, getAll, get and getOptional on the class (top-level) e.g. db.execute(), db.get() and they also expose these methods in lock and transaction contexts e.g. tx.get().

The top-level get functions were previously using the top-level execute function which would actually schedule a writeLock and then use the writeLock's context execute method. These methods should actually be using a readLock instead since those can execute concurrently with writeLocks. This method provides a read only execute method on the DB adapter class as shorthand for starting a readLock and using it's execute method.

This method is used to create the top-level get helpers here https://github.com/journeyapps/powersync-react-native-sdk/blob/fc2eb5d9bf74e71ee3cd7fe896ecba5ca4f27081/packages/powersync-sdk-react-native/src/db/adapters/react-native-quick-sqlite/RNQSDBAdapter.ts#L30

*/
private readOnlyExecute(sql: string, params?: any[]) {
return this.baseDB.readLock((ctx) => ctx.execute(sql, params));
}

/**
* Adds DB get utils to lock contexts and transaction contexts
* @param tx
Expand All @@ -68,23 +82,23 @@ export class RNQSDBAdapter extends BaseObserver<DBAdapterListener> implements DB
/**
* Execute a read-only query and return results
*/
async getAll<T>(sql: string, parameters?: any[]): Promise<T[]> {
getAll: async <T>(sql: string, parameters?: any[]): Promise<T[]> => {
const res = await tx.execute(sql, parameters);
return res.rows?._array ?? [];
},

/**
* Execute a read-only query and return the first result, or null if the ResultSet is empty.
*/
async getOptional<T>(sql: string, parameters?: any[]): Promise<T | null> {
getOptional: async <T>(sql: string, parameters?: any[]): Promise<T | null> => {
const res = await tx.execute(sql, parameters);
return res.rows?.item(0) ?? null;
},

/**
* Execute a read-only query and return the first result, error if the ResultSet is empty.
*/
async get<T>(sql: string, parameters?: any[]): Promise<T> {
get: async <T>(sql: string, parameters?: any[]): Promise<T> => {
const res = await tx.execute(sql, parameters);
const first = res.rows?.item(0);
if (!first) {
Expand Down
Loading
Loading