Merge pull request #593 from Nozbe/derxify3

Various fixes

CHANGELOG.md

@@ -6,7 +6,9 @@ All notable changes to this project will be documented in this file.
 
 ### ⚠️ Breaking
 
-(Low breakage risk)
+- `experimentalUseIncrementalIndexedDB` has been renamed to `useIncrementalIndexedDB`
+
+#### Low breakage risk
 
 - [adapters] Adapter API has changed from returning Promise to taking callbacks as the last argument. This won't affect you unless you call on adapter methods directly. `database.adapter` returns a new `DatabaseAdapterCompat` which has the same shape as old adapter API. You can use `database.adapter.underlyingAdapter` to get back `SQLiteAdapter` / `LokiJSAdapter`
 - [Collection] `Collection.fetchQuery` and `Collection.fetchCount` are removed. Please use `Query.fetch()` and `Query.fetchCount()`.
@@ -17,11 +19,10 @@ All notable changes to this project will be documented in this file.
   When enabled, database operations will block JavaScript thread. Adapter actions will resolve in the
   next microtask, which simplifies building flicker-free interfaces. Adapter will fall back to async
   operation when synchronous adapter is not available (e.g. when doing remote debugging)
-- [Model] Added experimental `model.experimentalSubscribe((isDeleted) => { ... })` method as a vanilla JS alternative to Rx based `model.observe()`. Unlike the latter, it does not notify the subscriber immediately upon subscription.
-- [Collection] Added internal `collection.experimentalSubscribe((changeSet) => { ... })` method as a vanilla JS alternative to Rx based `collection.changes` (you probably shouldn't be using this API anyway)
-- [Database] Added experimental `database.experimentalSubscribe(['table1', 'table2'], () => { ... })` method as a vanilla JS alternative to Rx-based `database.withChangesForTables()`. Unlike the latter, `experimentalSubscribe` notifies the subscriber only once after a batch that makes a change in multiple collections subscribed to. It also doesn't notify the subscriber immediately upon subscription, and doesn't send details about the changes, only a signal.
-- Added `experimentalDisableObserveCountThrottling()` to `@nozbe/watermelondb/observation/observeCount` that globally disables count observation throttling. We think that throttling on WatermelonDB level is not a good feature and will be removed in a future release - and will be better implemented on app level if necessary
-- [Query] Added experimental `query.experimentalSubscribe(records => { ... })`, `query.experimentalSubscribeWithColumns(['col1', 'col2'], records => { ... })`, and `query.experimentalSubscribeToCount(count => { ... })` methods
+- [LokiJS] Added new `onQuotaExceededError?: (error: Error) => void` option to `LokiJSAdapter` constructor.
+  This is called when underlying IndexedDB encountered a quota exceeded error (ran out of allotted disk space for app)
+  This means that app can't save more data or that it will fall back to using in-memory database only
+  Note that this only works when `useWebWorker: false`
 
 ### Changes
 
@@ -31,10 +32,19 @@ All notable changes to this project will be documented in this file.
 
 ### Fixes
 
+- Fixed a possible cause for "Record ID xxx#yyy was sent over the bridge, but it's not cached" error
 - [LokiJS] Fixed an issue preventing database from saving when using `experimentalUseIncrementalIndexedDB`
 - Fixed a potential issue when using `database.unsafeResetDatabase()`
 - [iOS] Fixed issue with clearing database under experimental synchronous mode
 
+### New features (Experimental)
+
+- [Model] Added experimental `model.experimentalSubscribe((isDeleted) => { ... })` method as a vanilla JS alternative to Rx based `model.observe()`. Unlike the latter, it does not notify the subscriber immediately upon subscription.
+- [Collection] Added internal `collection.experimentalSubscribe((changeSet) => { ... })` method as a vanilla JS alternative to Rx based `collection.changes` (you probably shouldn't be using this API anyway)
+- [Database] Added experimental `database.experimentalSubscribe(['table1', 'table2'], () => { ... })` method as a vanilla JS alternative to Rx-based `database.withChangesForTables()`. Unlike the latter, `experimentalSubscribe` notifies the subscriber only once after a batch that makes a change in multiple collections subscribed to. It also doesn't notify the subscriber immediately upon subscription, and doesn't send details about the changes, only a signal.
+- Added `experimentalDisableObserveCountThrottling()` to `@nozbe/watermelondb/observation/observeCount` that globally disables count observation throttling. We think that throttling on WatermelonDB level is not a good feature and will be removed in a future release - and will be better implemented on app level if necessary
+- [Query] Added experimental `query.experimentalSubscribe(records => { ... })`, `query.experimentalSubscribeWithColumns(['col1', 'col2'], records => { ... })`, and `query.experimentalSubscribeToCount(count => { ... })` methods
+
 ## 0.15 - 2019-11-08
 
 ### Highlights

docs-master/Installation.md

@@ -246,7 +246,7 @@ const adapter = new LokiJSAdapter({
   schema,
   // These two options are recommended for new projects:
   useWebWorker: false,
-  experimentalUseIncrementalIndexedDB: true,
+  useIncrementalIndexedDB: true,
   // It's recommended you implement this method:
   // onIndexedDBVersionChange: () => {
   //   // database was deleted in another browser tab (user logged out), so we must make sure we delete

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@nozbe/watermelondb",
   "description": "Build powerful React Native and React web apps that scale from hundreds to tens of thousands of records and remain fast",
-  "version": "0.16.0-7",
+  "version": "0.16.0-9",
   "scripts": {
     "build": "NODE_ENV=production node ./scripts/make.js",
     "dev": "NODE_ENV=development node ./scripts/make.js",

src/Collection/index.js

@@ -137,7 +137,7 @@ export default class Collection<Record: Model> {
     )
   }
 
-  changeSet(operations: CollectionChangeSet<Record>): void {
+  _applyChangesToCache(operations: CollectionChangeSet<Record>): void {
     operations.forEach(({ record, type }) => {
       if (type === CollectionChangeTypes.created) {
         record._isCommitted = true
@@ -146,7 +146,9 @@ export default class Collection<Record: Model> {
         this._cache.delete(record)
       }
     })
+  }
 
+  _notify(operations: CollectionChangeSet<Record>): void {
     this._subscribers.forEach(subscriber => {
       subscriber(operations)
     })

src/Database/ActionQueue.js

@@ -34,14 +34,14 @@ export default class ActionQueue implements ActionInterface {
         const queue = this._queue
         const current = queue[0]
         logger.warn(
-          `The action you're trying to perform (${description ||
+          `[WatermelonDB] The action you're trying to perform (${description ||
             'unnamed'}) can't be performed yet, because there are ${
             queue.length
           } actions in the queue. Current action: ${current.description ||
             'unnamed'}. Ignore this message if everything is working fine. But if your actions are not running, it's because the current action is stuck. Remember that if you're calling an action from an action, you must use subAction(). See docs for more details.`,
         )
-        logger.log(`Enqueued action:`, work)
-        logger.log(`Running action:`, current.work)
+        logger.log(`[WatermelonDB] Enqueued action:`, work)
+        logger.log(`[WatermelonDB] Running action:`, current.work)
       }
 
       this._queue.push({ work, resolve, reject, description })

src/Database/index.js

@@ -106,11 +106,15 @@ export default class Database {
 
     await this.adapter.batch(batchOperations)
 
-    // NOTE: Collections must be notified first to ensure that batched
-    // elements are marked as cached
+    // NOTE: We must make two passes to ensure all changes to caches are applied before subscribers are called
     Object.entries(changeNotifications).forEach(notification => {
       const [table, changeSet]: [TableName<any>, CollectionChangeSet<any>] = (notification: any)
-      this.collections.get(table).changeSet(changeSet)
+      this.collections.get(table)._applyChangesToCache(changeSet)
+    })
+
+    Object.entries(changeNotifications).forEach(notification => {
+      const [table, changeSet]: [TableName<any>, CollectionChangeSet<any>] = (notification: any)
+      this.collections.get(table)._notify(changeSet)
     })
 
     const affectedTables = Object.keys(changeNotifications)

src/Database/test.js

@@ -303,6 +303,33 @@ describe('Observation', () => {
     expect(subscriber1).toHaveBeenCalledTimes(1)
     unsubscribe1()
   })
+  it('has new objects cached before calling subscribers (regression test)', async () => {
+    const { database, projects, tasks } = mockDatabase({ actionsEnabled: true })
+
+    const project = projects.prepareCreate()
+    const task = tasks.prepareCreate(t => {
+      t.project.set(project)
+    })
+
+    let observerCalled = 0
+    let taskPromise = null
+    const observer = jest.fn(() => {
+      observerCalled += 1
+      if (observerCalled === 1) {
+        // nothing happens
+      } else if (observerCalled === 2) {
+        taskPromise = tasks.find(task.id)
+      }
+    })
+    database.withChangesForTables(['mock_projects']).subscribe(observer)
+    expect(observer).toHaveBeenCalledTimes(1)
+
+    await database.action(() => database.batch(project, task))
+    expect(observer).toHaveBeenCalledTimes(2)
+
+    // check if task is already cached
+    expect(await taskPromise).toBe(task)
+  })
 })
 
 const delayPromise = () => new Promise(resolve => setTimeout(resolve, 100))

src/adapters/__tests__/commonTests.js

@@ -37,9 +37,15 @@ export default () => [
       // expect(() => makeAdapter({})).toThrowError(/missing migrations/)
 
       expect(() => makeAdapter({ migrationsExperimental: [] })).toThrow(
-        /migrationsExperimental has been renamed/,
+        /`migrationsExperimental` option has been renamed to `migrations`/,
       )
 
+      if (AdapterClass.name === 'LokiJSAdapter') {
+        expect(() => makeAdapter({ experimentalUseIncrementalIndexedDB: false })).toThrow(
+          /LokiJSAdapter `experimentalUseIncrementalIndexedDB` option has been renamed/,
+        )
+      }
+
       expect(() => adapterWithMigrations({ migrations: [] })).toThrow(/use schemaMigrations()/)
 
       // OK migrations passed

src/adapters/common.js

@@ -57,7 +57,7 @@ export function sanitizeQueryResult(
 export function devSetupCallback(result: Result<any>): void {
   if (result.error) {
     logger.error(
-      `[DB] Uh-oh. Database failed to load, we're in big trouble. This might happen if you didn't set up native code correctly (iOS, Android), or if you didn't recompile native app after WatermelonDB update. It might also mean that IndexedDB or SQLite refused to open.`,
+      `[WatermelonDB] Uh-oh. Database failed to load, we're in big trouble. This might happen if you didn't set up native code correctly (iOS, Android), or if you didn't recompile native app after WatermelonDB update. It might also mean that IndexedDB or SQLite refused to open.`,
       result.error,
     )
   }

src/adapters/lokijs/index.js

@@ -1,7 +1,7 @@
 // @flow
 
 import type { LokiMemoryAdapter } from 'lokijs'
-import { invariant } from '../../utils/common'
+import { invariant, logger } from '../../utils/common'
 import type { ResultCallback } from '../../utils/fp/Result'
 
 import type { RecordId } from '../../Model'
@@ -35,11 +35,15 @@ export type LokiAdapterOptions = $Exact<{
   // (true by default) Although web workers may have some throughput benefits, disabling them
   // may lead to lower memory consumption, lower latency, and easier debugging
   useWebWorker?: boolean,
-  experimentalUseIncrementalIndexedDB?: boolean,
-  // Called when internal IDB version changed (most likely the database was deleted in another browser tab)
+  useIncrementalIndexedDB?: boolean,
+  // Called when internal IndexedDB version changed (most likely the database was deleted in another browser tab)
   // Pass a callback to force log out in this copy of the app as well
   // Note that this only works when using incrementalIDB and not using web workers
   onIndexedDBVersionChange?: () => void,
+  // Called when underlying IndexedDB encountered a quota exceeded error (ran out of allotted disk space for app)
+  // This means that app can't save more data or that it will fall back to using in-memory database only
+  // Note that this only works when `useWebWorker: false`
+  onQuotaExceededError?: (error: Error) => void,
   // -- internal --
   _testLokiAdapter?: LokiMemoryAdapter,
 }>
@@ -64,10 +68,23 @@ export default class LokiJSAdapter implements DatabaseAdapter {
     this._dbName = dbName
 
     if (process.env.NODE_ENV !== 'production') {
+      if (!('useWebWorker' in options)) {
+        logger.warn(
+          'LokiJSAdapter `useWebWorker` option will become required in a future version of WatermelonDB. Pass `{ useWebWorker: false }` to adopt the new behavior, or `{ useWebWorker: true }` to supress this warning with no changes',
+        )
+      }
+      if (!('useIncrementalIndexedDB' in options)) {
+        logger.warn(
+          'LokiJSAdapter `useIncrementalIndexedDB` option will become required in a future version of WatermelonDB. Pass `{ useIncrementalIndexedDB: true }` to adopt the new behavior, or `{ useIncrementalIndexedDB: false }` to supress this warning with no changes',
+        )
+      }
       invariant(
-        // $FlowFixMe
-        options.migrationsExperimental === undefined,
-        'LokiJSAdapter migrationsExperimental has been renamed to migrations',
+        !('migrationsExperimental' in options),
+        'LokiJSAdapter `migrationsExperimental` option has been renamed to `migrations`',
+      )
+      invariant(
+        !('experimentalUseIncrementalIndexedDB' in options),
+        'LokiJSAdapter `experimentalUseIncrementalIndexedDB` option has been renamed to `useIncrementalIndexedDB`',
       )
       validateAdapter(this)
     }

src/adapters/lokijs/worker/executor.js

@@ -33,9 +33,11 @@ export default class LokiExecutor {
 
   loki: Loki
 
-  experimentalUseIncrementalIndexedDB: boolean
+  useIncrementalIndexedDB: boolean
 
-  onIndexedDBVersionChange: ?(() => void)
+  onIndexedDBVersionChange: ?() => void
+
+  onQuotaExceededError: ?(error: Error) => void
 
   _testLokiAdapter: ?LokiMemoryAdapter
 
@@ -46,8 +48,9 @@ export default class LokiExecutor {
     this.dbName = dbName
     this.schema = schema
     this.migrations = migrations
-    this.experimentalUseIncrementalIndexedDB = options.experimentalUseIncrementalIndexedDB || false
+    this.useIncrementalIndexedDB = options.useIncrementalIndexedDB || false
     this.onIndexedDBVersionChange = options.onIndexedDBVersionChange
+    this.onQuotaExceededError = options.onQuotaExceededError
     this._testLokiAdapter = _testLokiAdapter
   }
 
@@ -197,7 +200,7 @@ export default class LokiExecutor {
     await deleteDatabase(this.loki)
 
     this.cachedRecords.clear()
-    logger.log('[DB][Worker] Database is now reset')
+    logger.log('[WatermelonDB][Loki] Database is now reset')
 
     await this._openDatabase()
     this._setUpSchema()
@@ -233,20 +236,21 @@ export default class LokiExecutor {
   // *** Internals ***
 
   async _openDatabase(): Promise<void> {
-    logger.log('[DB][Worker] Initializing IndexedDB')
+    logger.log('[WatermelonDB][Loki] Initializing IndexedDB')
 
     this.loki = await newLoki(
       this.dbName,
       this._testLokiAdapter,
-      this.experimentalUseIncrementalIndexedDB,
+      this.useIncrementalIndexedDB,
       this.onIndexedDBVersionChange,
+      this.onQuotaExceededError,
     )
 
-    logger.log('[DB][Worker] Database loaded')
+    logger.log('[WatermelonDB][Loki] Database loaded')
   }
 
   _setUpSchema(): void {
-    logger.log('[DB][Worker] Setting up schema')
+    logger.log('[WatermelonDB][Loki] Setting up schema')
 
     // Add collections
     values(this.schema.tables).forEach(tableSchema => {
@@ -262,7 +266,7 @@ export default class LokiExecutor {
     // Set database version
     this._databaseVersion = this.schema.version
 
-    logger.log('[DB][Worker] Database collections set up')
+    logger.log('[WatermelonDB][Loki] Database collections set up')
   }
 
   _addCollection(tableSchema: TableSchema): void {
@@ -295,28 +299,32 @@ export default class LokiExecutor {
     if (dbVersion === schemaVersion) {
       // All good!
     } else if (dbVersion === 0) {
-      logger.log('[DB][Worker] Empty database, setting up')
+      logger.log('[WatermelonDB][Loki] Empty database, setting up')
       await this.unsafeResetDatabase()
     } else if (dbVersion > 0 && dbVersion < schemaVersion) {
-      logger.log('[DB][Worker] Database has old schema version. Migration is required.')
+      logger.log('[WatermelonDB][Loki] Database has old schema version. Migration is required.')
       const migrationSteps = this._getMigrationSteps(dbVersion)
 
       if (migrationSteps) {
-        logger.log(`[DB][Worker] Migrating from version ${dbVersion} to ${this.schema.version}...`)
+        logger.log(
+          `[WatermelonDB][Loki] Migrating from version ${dbVersion} to ${this.schema.version}...`,
+        )
         try {
           await this._migrate(migrationSteps)
         } catch (error) {
-          logger.error('[DB][Worker] Migration failed', error)
+          logger.error('[WatermelonDB][Loki] Migration failed', error)
           throw error
         }
       } else {
         logger.warn(
-          '[DB][Worker] Migrations not available for this version range, resetting database instead',
+          '[WatermelonDB][Loki] Migrations not available for this version range, resetting database instead',
         )
         await this.unsafeResetDatabase()
       }
     } else {
-      logger.warn('[DB][Worker] Database has newer version than app schema. Resetting database.')
+      logger.warn(
+        '[WatermelonDB][Loki] Database has newer version than app schema. Resetting database.',
+      )
       await this.unsafeResetDatabase()
     }
   }
@@ -349,7 +357,7 @@ export default class LokiExecutor {
     // Set database version
     this._databaseVersion = this.schema.version
 
-    logger.log(`[DB][Worker] Migration successful`)
+    logger.log(`[WatermelonDB][Loki] Migration successful`)
   }
 
   _executeCreateTableMigration({ schema }: CreateTableMigrationStep): void {