fix: disallow chaining AlterColumnBuilder operations (kysely-org#467)

soanvig · May 28, 2023 · e7eb9b3 · e7eb9b3
1 parent bf0cafc
commit e7eb9b3
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 37 deletions.
diff --git a/src/operation-node/alter-column-node.ts b/src/operation-node/alter-column-node.ts
@@ -1,8 +1,6 @@
 import { OperationNode } from './operation-node.js'
 import { freeze } from '../util/object-utils.js'
 import { ColumnNode } from './column-node.js'
-import { DataTypeNode } from './data-type-node.js'
-import { ValueNode } from './value-node.js'
 import { RawNode } from './raw-node.js'
 
 export type AlterColumnNodeProps = Omit<AlterColumnNode, 'kind' | 'column'>
@@ -26,20 +24,11 @@ export const AlterColumnNode = freeze({
     return node.kind === 'AlterColumnNode'
   },
 
-  create(column: string): AlterColumnNode {
+  create<T extends keyof AlterColumnNode>(column: string, prop: T, value: Required<AlterColumnNode>[T]): AlterColumnNode {
     return freeze({
       kind: 'AlterColumnNode',
       column: ColumnNode.create(column),
-    })
-  },
-
-  cloneWith(
-    node: AlterColumnNode,
-    props: AlterColumnNodeProps
-  ): AlterColumnNode {
-    return freeze({
-      ...node,
-      ...props,
+      [prop]: value
     })
   },
 })
diff --git a/src/schema/alter-column-builder.ts b/src/schema/alter-column-builder.ts
@@ -10,49 +10,39 @@ import {
 } from '../parser/default-value-parser.js'
 
 export class AlterColumnBuilder {
-  protected readonly alterColumnNode: AlterColumnNode
+  protected column: string
 
-  constructor(alterColumnNode: AlterColumnNode) {
-    this.alterColumnNode = alterColumnNode
+  constructor(column: string) {
+    this.column = column
   }
 
   setDataType(dataType: DataTypeExpression): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        dataType: parseDataTypeExpression(dataType),
-      })
+      AlterColumnNode.create(this.column, 'dataType', parseDataTypeExpression(dataType))
     )
   }
 
   setDefault(value: DefaultValueExpression): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        setDefault: parseDefaultValueExpression(value),
-      })
+      AlterColumnNode.create(this.column, 'setDefault', parseDefaultValueExpression(value))
     )
   }
 
   dropDefault(): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        dropDefault: true,
-      })
+      AlterColumnNode.create(this.column, 'dropDefault', true)
     )
   }
 
   setNotNull(): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        setNotNull: true,
-      })
+      AlterColumnNode.create(this.column, 'setNotNull', true)
     )
   }
 
   dropNotNull(): AlteredColumnBuilder {
     return new AlteredColumnBuilder(
-      AlterColumnNode.cloneWith(this.alterColumnNode, {
-        dropNotNull: true,
-      })
+      AlterColumnNode.create(this.column, 'dropNotNull', true)
     )
   }
 
@@ -66,20 +56,30 @@ export class AlterColumnBuilder {
 }
 
 /**
- * Allows us to force consumers to do something, anything, when altering a column.
+ * Allows us to force consumers to do something, anything, when altering a column,
+ * and also disallows chaining more methods on AlterColumnBuilder, because
+ * SQL syntax for chaining ALTER COLUMN operations would be not WYSIWYG
  *
  * Basically, deny the following:
  *
  * ```ts
  * db.schema.alterTable('person').alterColumn('age', (ac) => ac)
  * ```
+ * 
+ * ```ts
+ * db.schema.alterTable('person').alterColumn('age', (ac) => ac.dropNotNull().setNotNull())
+ * ```
  *
  * Which would now throw a compilation error, instead of a runtime error.
  */
-export class AlteredColumnBuilder
-  extends AlterColumnBuilder
-  implements OperationNodeSource
+export class AlteredColumnBuilder implements OperationNodeSource
 {
+  protected readonly alterColumnNode: AlterColumnNode
+
+  constructor(alterColumnNode: AlterColumnNode) {
+    this.alterColumnNode = alterColumnNode
+  }
+
   toOperationNode(): AlterColumnNode {
     return this.alterColumnNode
   }

diff --git a/src/schema/alter-table-builder.ts b/src/schema/alter-table-builder.ts
@@ -71,7 +71,7 @@ export class AlterTableBuilder implements ColumnAlteringInterface {
     alteration: AlterColumnBuilderCallback
   ): AlterTableColumnAlteringBuilder {
     const builder = alteration(
-      new AlterColumnBuilder(AlterColumnNode.create(column))
+      new AlterColumnBuilder(column)
     )
 
     return new AlterTableColumnAlteringBuilder({
@@ -292,7 +292,7 @@ export class AlterTableColumnAlteringBuilder
     alteration: AlterColumnBuilderCallback
   ): AlterTableColumnAlteringBuilder {
     const builder = alteration(
-      new AlterColumnBuilder(AlterColumnNode.create(column))
+      new AlterColumnBuilder(column)
     )
 
     return new AlterTableColumnAlteringBuilder({