kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
9.81k stars 249 forks source link

Is Type incorrect in `UpdateQueryNode` #1067

Closed zce closed 3 days ago

zce commented 3 days ago

https://github.com/kysely-org/kysely/blob/f353fc6d333b70424e1819395eac4a81c26e4df4/src/operation-node/column-update-node.ts#L6-L7

Should the types here be ColumnNode and ValueNode?

export interface ColumnUpdateNode extends OperationNode {
  readonly kind: 'ColumnUpdateNode'
  readonly column: ColumnNode
  readonly value: ValueNode
}

in inspector:

{
  kind: 'ColumnUpdateNode',
  column: {
    kind: 'ColumnNode',
    column: { kind: 'IdentifierNode', name: 'type' }
  },
  value: { kind: 'ValueNode', value: 'course' }
}

I’m not sure if there is any special purpose behind using base type, if not I think I can submit a PR to fix it

koskimas commented 3 days ago

ColumnNode and ValueNode implement the OperationNode interface. There's no mistake there.

zce commented 3 days ago

thanks for prompt responses! I just noticed there is no automatic type narrowing when using it:

class UpdatedAtPlugin implements KyselyPlugin {
  constructor(private readonly column: string = 'updated_at') {}

  transformQuery(args: PluginTransformQueryArgs) {
    if (args.node.kind !== 'UpdateQueryNode') return args.node
    if (args.node.updates?.some(item => item.column.kind === 'ColumnNode' && (item.column as ColumnNode).column.name === this.column)) return args.node
    const updatedAt = ColumnUpdateNode.create(ColumnNode.create(this.column), ValueNode.create(new Date().toISOString()))
    return UpdateQueryNode.cloneWithUpdates(args.node, [updatedAt])
  }

  transformResult(args: PluginTransformResultArgs) {
    return Promise.resolve(args.result)
  }
}
igalklebanov commented 3 days ago

Hey 👋

Try using these:

UpdateQueryNode.is(args.node)
ColumnNode.is(item.column)