Add destroyColumn, renameColumn, and destroyTable migration steps

CHANGELOG-Unreleased.md

@@ -9,6 +9,10 @@
 ### New features
 
 - Added `Database#experimentalIsVerbose` option
+- New migration steps available:
+  - `destroyColumn` (see docs for limitations)
+  - `renameColumn` (see docs for limitations)
+  - `destroyTable`
 
 ### Fixes
 

docs-website/docs/docs/Advanced/Migrations.md

@@ -174,7 +174,12 @@ schemaMigrations({
 
 - `createTable({ name: 'table_name', columns: [ ... ] })` - same API as `tableSchema()`
 - `addColumns({ table: 'table_name', columns: [ ... ] })` - you can add one or multiple columns to an existing table. The columns table has the same format as in schema definitions
-- Other types of migrations (e.g. deleting or renaming tables and columns) are not yet implemented. See [`migrations/index.js`](https://github.com/Nozbe/WatermelonDB/blob/master/src/Schema/migrations/index.js). Please contribute!
+- `destroyColumn({ table: 'table_name', column: 'column_name' })` - removes column from table
+  - ⚠️ Important! This migration requires sqlite 3.35.0, available since iOS 15 and Android 14 (in non-JSI mode). `lokijs` adapter, Android in JSI mode, and sqlite-node are not affected by this limitation.
+- `renameColumn({ table: 'table_name', from: 'old_column_name', to: 'new_column_name' })` - renames column in table
+  - ⚠️ Important! This migration requires sqlite 3.25.0, available since iOS 13 and Android 11 (in non-JSI mode). `lokijs` adapter, Android in JSI mode, and sqlite-node are not affected by this limitation.
+- `destroyTable({ table: 'table_name' })` - removes table
+- Other types of migrations (e.g. renaming tables, changing indices) are not yet implemented. See [`migrations/index.js`](https://github.com/Nozbe/WatermelonDB/blob/master/src/Schema/migrations/index.js). Please contribute!
 
 ## Database reseting and other edge cases
 

src/RawRecord/index.d.ts

@@ -14,7 +14,7 @@ type _RawRecord = {
 }
 
 // Raw object representing a model record. A RawRecord is guaranteed by the type system
-// to be safe to use (sanitied with `sanitizedRaw`):
+// to be safe to use (sanitized with `sanitizedRaw`):
 // - it has exactly the fields described by TableSchema (+ standard fields)
 // - every field is exactly the type described by ColumnSchema (string, number, or boolean)
 // - … and the same optionality (will not be null unless isOptional: true)

src/RawRecord/index.js

@@ -20,7 +20,7 @@ type _RawRecord = {
 }
 
 // Raw object representing a model record. A RawRecord is guaranteed by the type system
-// to be safe to use (sanitied with `sanitizedRaw`):
+// to be safe to use (sanitized with `sanitizedRaw`):
 // - it has exactly the fields described by TableSchema (+ standard fields)
 // - every field is exactly the type described by ColumnSchema (string, number, or boolean)
 // - … and the same optionality (will not be null unless isOptional: true)

src/Schema/index.js

@@ -98,7 +98,7 @@ export function appSchema({ version, tables: tableList, unsafeSql }: AppSchemaSp
   return { version, tables, unsafeSql }
 }
 
-const validateName = (name: string) => {
+export const validateName = (name: string) => {
   if (process.env.NODE_ENV !== 'production') {
     invariant(
       !['id', '_changed', '_status', 'local_storage'].includes(name.toLowerCase()),

src/Schema/migrations/getSyncChanges/index.js

@@ -1,7 +1,5 @@
 // @flow
-
-import { unique, groupBy, toPairs, pipe, unnest } from '../../../utils/fp'
-import type { CreateTableMigrationStep, AddColumnsMigrationStep, SchemaMigrations } from '../index'
+import type { SchemaMigrations } from '../index'
 import type { TableName, ColumnName, SchemaVersion } from '../../index'
 import { tableName } from '../../index'
 import { stepsForMigration } from '../stepsForMigration'
@@ -32,40 +30,58 @@ export default function getSyncChanges(
     return null
   }
 
+  const createdTables: Set<TableName<any>> = new Set()
+  const createdColumns: Map<string, Set<ColumnName>> = new Map()
+
   steps.forEach((step) => {
     invariant(
-      ['create_table', 'add_columns', 'sql'].includes(step.type),
+      [
+        'create_table',
+        'add_columns',
+        'destroy_column',
+        'rename_column',
+        'destroy_table',
+        'sql',
+      ].includes(step.type),
       `Unknown migration step type ${step.type}. Can not perform migration sync. This most likely means your migrations are defined incorrectly. It could also be a WatermelonDB bug.`,
     )
-  })
 
-  // $FlowFixMe
-  const createTableSteps: CreateTableMigrationStep[] = steps.filter(
-    (step) => step.type === 'create_table',
-  )
-  const createdTables = createTableSteps.map((step) => step.schema.name)
-
-  // $FlowFixMe
-  const addColumnSteps: AddColumnsMigrationStep[] = steps.filter(
-    (step) => step.type === 'add_columns',
-  )
-  const allAddedColumns = addColumnSteps
-    .filter((step) => !createdTables.includes(step.table))
-    .map(({ table, columns }) => columns.map(({ name }) => ({ table, name })))
+    if (step.type === 'create_table') {
+      createdTables.add(step.schema.name)
+    } else if (step.type === 'add_columns') {
+      if (createdTables.has(step.table)) {
+        return
+      }
+      const columns = createdColumns.get(step.table) || new Set()
+      step.columns.forEach((column) => {
+        columns.add(column.name)
+      })
+      createdColumns.set(step.table, columns)
+    } else if (step.type === 'destroy_table') {
+      createdTables.delete(step.table)
+      createdColumns.delete(step.table)
+    } else if (step.type === 'destroy_column') {
+      const columns = createdColumns.get(step.table)
+      if (columns) {
+        columns.delete(step.column)
+      }
+    } else if (step.type === 'rename_column') {
+      const columns = createdColumns.get(step.table)
+      if (columns && columns.has(step.from)) {
+        columns.delete(step.from)
+        columns.add(step.to)
+      }
+    }
+  })
 
-  const columnsByTable = pipe(
-    unnest,
-    groupBy(({ table }) => table),
-    toPairs,
-  )(allAddedColumns)
-  const addedColumns = columnsByTable.map(([table, columnDefs]) => ({
+  const columnsByTable = Array.from(createdColumns.entries()).map(([table, columns]) => ({
     table: tableName(table),
-    columns: unique(columnDefs.map(({ name }) => name)),
+    columns: Array.from(columns),
   }))
 
   return {
     from: fromVersion,
-    tables: unique(createdTables),
-    columns: addedColumns,
+    tables: Array.from(createdTables),
+    columns: columnsByTable,
   }
 }

src/Schema/migrations/getSyncChanges/test.js

@@ -1,5 +1,13 @@
 import getSyncChanges from './index'
-import { schemaMigrations, createTable, addColumns, unsafeExecuteSql } from '../index'
+import {
+  schemaMigrations,
+  createTable,
+  addColumns,
+  unsafeExecuteSql,
+  renameColumn,
+  destroyColumn,
+  destroyTable,
+} from '../index'
 
 const createCommentsTable = createTable({
   name: 'comments',
@@ -8,6 +16,13 @@ const createCommentsTable = createTable({
     { name: 'body', type: 'string' },
   ],
 })
+const addColumnsToPostsTable = addColumns({
+  table: 'posts',
+  columns: [
+    { name: 'subtitle', type: 'string', isOptional: true },
+    { name: 'is_pinned', type: 'boolean' },
+  ],
+})
 
 const test = (migrations, from, to) => getSyncChanges(schemaMigrations({ migrations }), from, to)
 const testSteps = (steps) =>
@@ -21,20 +36,14 @@ describe('getSyncChanges', () => {
     expect(testSteps([])).toEqual({ from: 1, tables: [], columns: [] })
   })
   it('returns created tables', () => {
-    expect(testSteps([createCommentsTable])).toEqual({ from: 1, tables: ['comments'], columns: [] })
+    expect(testSteps([createCommentsTable])).toEqual({
+      from: 1,
+      tables: ['comments'],
+      columns: [],
+    })
   })
   it('returns added columns', () => {
-    expect(
-      testSteps([
-        addColumns({
-          table: 'posts',
-          columns: [
-            { name: 'subtitle', type: 'string', isOptional: true },
-            { name: 'is_pinned', type: 'boolean' },
-          ],
-        }),
-      ]),
-    ).toEqual({
+    expect(testSteps([addColumnsToPostsTable])).toEqual({
       from: 1,
       tables: [],
       columns: [{ table: 'posts', columns: ['subtitle', 'is_pinned'] }],
@@ -77,6 +86,73 @@ describe('getSyncChanges', () => {
       columns: [],
     })
   })
+  it('skips added tables if subsequently destroyed', () => {
+    expect(
+      testSteps([
+        createCommentsTable,
+        destroyTable({ table: 'comments' }),
+        destroyTable({ table: 'unrelated' }),
+      ]),
+    ).toEqual({
+      from: 1,
+      tables: [],
+      columns: [],
+    })
+  })
+  it('skips added columns if subsequently destroyed', () => {
+    expect(
+      testSteps([
+        addColumnsToPostsTable,
+        destroyColumn({ table: 'posts', column: 'is_pinned' }),
+        destroyColumn({ table: 'posts', column: 'unrelated' }),
+        destroyColumn({ table: 'unrelated', column: 'is_pinned' }),
+      ]),
+    ).toEqual({
+      from: 1,
+      tables: [],
+      columns: [{ table: 'posts', columns: ['subtitle'] }],
+    })
+  })
+  it('skips added columns if parent table subsequently destroyed', () => {
+    expect(
+      testSteps([
+        addColumnsToPostsTable,
+        destroyTable({ table: 'posts' }),
+        destroyTable({ table: 'unrelated' }),
+      ]),
+    ).toEqual({
+      from: 1,
+      tables: [],
+      columns: [],
+    })
+  })
+  it('renames added columns if needed', () => {
+    expect(
+      testSteps([
+        addColumnsToPostsTable,
+        renameColumn({ table: 'posts', from: 'is_pinned', to: 'is_starred' }),
+        // not needed
+        renameColumn({ table: 'posts', from: 'foo', to: 'bar' }),
+      ]),
+    ).toEqual({
+      from: 1,
+      tables: [],
+      columns: [{ table: 'posts', columns: ['subtitle', 'is_starred'] }],
+    })
+  })
+  it('skips added columns if subsequently destroyed, after renaming', () => {
+    expect(
+      testSteps([
+        addColumnsToPostsTable,
+        renameColumn({ table: 'posts', from: 'is_pinned', to: 'is_starred' }),
+        destroyColumn({ table: 'posts', column: 'is_starred' }),
+      ]),
+    ).toEqual({
+      from: 1,
+      tables: [],
+      columns: [{ table: 'posts', columns: ['subtitle'] }],
+    })
+  })
   it('skips duplicates', () => {
     // technically, a duplicate createTable or addColumn would crash
     // but this is in case future migration types could do something like it
@@ -214,7 +290,11 @@ describe('getSyncChanges', () => {
       tables: [],
       columns: [{ table: 'attachment_versions', columns: ['reactions'] }],
     })
-    expect(test(bigMigrations, 9, 10)).toEqual({ from: 9, tables: [], columns: [] })
+    expect(test(bigMigrations, 9, 10)).toEqual({
+      from: 9,
+      tables: [],
+      columns: [],
+    })
     expect(test(bigMigrations, 10, 10)).toEqual(null)
   })
   it(`fails on incorrect migrations`, () => {
@@ -225,12 +305,9 @@ describe('getSyncChanges', () => {
     const possibleFutureTypes = [
       'broken',
       'rename_table',
-      'rename_column',
       'add_column_index',
       'make_column_optional',
       'make_column_required',
-      'destroy_table',
-      'destroy_column',
     ]
     possibleFutureTypes.forEach((type) => {
       expect(() => testSteps([{ type }])).toThrow('Unknown migration step type')

src/Schema/migrations/index.d.ts

@@ -1,5 +1,12 @@
 import type { $RE, $Exact } from '../../types'
-import type { ColumnSchema, TableName, TableSchema, TableSchemaSpec, SchemaVersion } from '../index'
+import type {
+  ColumnName,
+  ColumnSchema,
+  TableName,
+  TableSchema,
+  TableSchemaSpec,
+  SchemaVersion,
+} from '../index'
 
 export type CreateTableMigrationStep = $RE<{
   type: 'create_table'
@@ -13,12 +20,39 @@ export type AddColumnsMigrationStep = $RE<{
   unsafeSql?: (_: string) => string
 }>
 
+export type DestroyColumnMigrationStep = $RE<{
+  type: 'destroy_column'
+  table: TableName<any>
+  column: ColumnName
+  unsafeSql?: (_: string) => string
+}>
+
+export type RenameColumnMigrationStep = $RE<{
+  type: 'rename_column'
+  table: TableName<any>
+  from: ColumnName
+  to: ColumnName
+  unsafeSql?: (_: string) => string
+}>
+
+export type DestroyTableMigrationStep = $RE<{
+  type: 'destroy_table'
+  table: TableName<any>
+  unsafeSql?: (_: string) => string
+}>
+
 export type SqlMigrationStep = $RE<{
   type: 'sql'
   sql: string
 }>
 
-export type MigrationStep = CreateTableMigrationStep | AddColumnsMigrationStep | SqlMigrationStep
+export type MigrationStep =
+  | CreateTableMigrationStep
+  | AddColumnsMigrationStep
+  | SqlMigrationStep
+  | DestroyColumnMigrationStep
+  | RenameColumnMigrationStep
+  | DestroyTableMigrationStep
 
 type Migration = $RE<{
   toVersion: SchemaVersion
@@ -50,4 +84,36 @@ export function addColumns({
   unsafeSql?: (_: string) => string
 }>): AddColumnsMigrationStep
 
+/** Requires sqlite 3.35.0 (iOS 15 / Android 14) */
+export function destroyColumn({
+  table,
+  column,
+  unsafeSql,
+}: $Exact<{
+  table: TableName<any>
+  column: ColumnName
+  unsafeSql?: (_: string) => string
+}>): DestroyColumnMigrationStep
+
+/** Requires sqlite 3.25.0 (iOS 13 / Android 11) */
+export function renameColumn({
+  table,
+  from,
+  to,
+  unsafeSql,
+}: $Exact<{
+  table: TableName<any>
+  from: string
+  to: string
+  unsafeSql?: (_: string) => string
+}>): RenameColumnMigrationStep
+
+export function destroyTable({
+  table,
+  unsafeSql,
+}: $Exact<{
+  table: TableName<any>
+  unsafeSql?: (_: string) => string
+}>): DestroyTableMigrationStep
+
 export function unsafeExecuteSql(sql: string): SqlMigrationStep