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

[BUG]: Method .returning() working incorrect with .get() method #158

Closed
Bulbang opened this issue Feb 3, 2023 · 8 comments
Closed

[BUG]: Method .returning() working incorrect with .get() method #158

Bulbang opened this issue Feb 3, 2023 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@Bulbang
Copy link
Contributor

Bulbang commented Feb 3, 2023

What version of drizzle-orm are you using?

0.17.4

Describe the Bug

Method .returning() is not working properly with method .get()

Full context you can find in these files:

schema.ts
SqliteJson.ts

schema.ts

import { sql } from 'drizzle-orm'
import { sqliteTable, integer, text } from 'drizzle-orm/sqlite-core'
import { json } from './SqliteJson'

export const articles = sqliteTable('articles', {
    id: integer('id').primaryKey(),
    slug: text('slug').notNull(),
    title: text('title').notNull(),
    meta_title: text('meta_title').notNull(),
    meta_description: text('meta_description').notNull(),
    blocks: json<{ [key: string]: unknown }>('blocks').notNull(),
    created_at: integer('created_at', { mode: 'timestamp' }).notNull(),
    updated_at: integer('updated_at', { mode: 'timestamp' }).notNull(),
    file_id: text('file_id'),
    hidden: integer('hidden').default(0),
    podcasts: json<{ [url: string]: string }>('podcasts').default(sql`NULL`),
})

SqliteJson.ts

import { ColumnConfig } from 'drizzle-orm'
import { AnySQLiteTable } from 'drizzle-orm/sqlite-core'
import {
    SQLiteColumn,
} from 'drizzle-orm/sqlite-core/columns'
import { ColumnBuilderConfig } from 'drizzle-orm/column-builder'
import { SQLiteColumnBuilder } from 'drizzle-orm/sqlite-core/columns/common';

export class SQLiteJsonBuilder<
    TData,
> extends SQLiteColumnBuilder<
    ColumnBuilderConfig<{ data: TData; driverParam: string }>
> {
    // protected override $sql!: 'SQLiteJsonBuilder';
    build<TTableName extends string>(
        table: AnySQLiteTable<{ name: TTableName }>,
    ): SQLiteJson<TTableName, TData> {
        return new SQLiteJson(table, this.config)
    }
}

export class SQLiteJson<
    TTableName extends string,
    TData,
> extends SQLiteColumn<
    ColumnConfig<{ tableName: TTableName; data: TData; driverParam: string }>
> {
    protected override $sqliteColumnBrand!: 'SQLiteJson'

    constructor(
        table: AnySQLiteTable<{ name: TTableName }>,
        builder: SQLiteJsonBuilder<TData>['config'],
    ) {
        super(table, builder)
    }

    getSQLType(): string {
        return 'json'
    }

    override mapFromDriverValue(value: string): TData {
        if (typeof value === 'string') {
			try {
				return JSON.parse(value);
			} catch (e) {
				return value as unknown as TData;
			}
		}
		return value;
    }

    override mapToDriverValue(value: TData): string {
        return JSON.stringify(value)
    }
}

export const json = <T>(name: string) => {
    return new SQLiteJsonBuilder<T>(name)
}

Code with bug

const newArticle = {
    title: 'no-img2',
    meta_title: 'biba',
    meta_description: 'sadasd',
    blocks: {
        props: {},
    },
    slug: 'cvbcvbc',
    created_at: new Date(),
    updated_at: new Date(),
}

const query = database
    .insert(articles)
    .values(newArticle)
    .returning()

const result = query.get()
console.log(result);

Log:

{
  id: undefined,
  slug: undefined,
  title: undefined,
  meta_title: undefined,
  meta_description: undefined,
  blocks: undefined,
  created_at: Invalid Date,
  updated_at: Invalid Date,
  file_id: undefined,
  hidden: undefined,
  podcasts: undefined
}

But when I changed const result = query.get() to const result = query.all() it seems to be working well

Log with .all() method:

[
  {
    id: 26,
    slug: 'cvbcvbc',
    title: 'no-img2',
    meta_title: 'biba',
    meta_description: 'sadasd',
    blocks: { props: {} },
    created_at: 2023-02-03T08:57:41.430Z,
    updated_at: 2023-02-03T08:57:41.430Z,
    file_id: null,
    hidden: 0,
    podcasts: null
  }
]
@Bulbang Bulbang added the bug Something isn't working label Feb 3, 2023
@Sandy-Garrido
Copy link

I can confirm, I was also seeing this issue! I have to .all()[0] for the equivalent

AndriiSherman added a commit that referenced this issue Feb 5, 2023
1. Issue link - #158
2. Add test cases for SQLite Proxy Driver
@AndriiSherman
Copy link
Member

@Bulbang @Sandy-Garrido this issue was fixed and released in 0.17.7

Have added additional test cases for .returning().get() mappings, you can check them here

Please confirm it's worked for you and I'll close this issue

@Sandy-Garrido
Copy link

Hmm 🤔 it doesn't look to have been fixed. I triple checked the version etc.

export const createApiKey = (): ApiKey => {
  const uuid = uuidv4();
  const apiKey = db
    .insert(apiKeys)
    .values({
      apiKey: uuid,
      createdAt: new Date()
    })
    .returning();

  console.log(apiKey.all()[0]);
  console.log(apiKey.get());
  return apiKey.all()[0];
};
[
  {
    id: 76,
    apiKey: "2023-02-05T17:07:22.140Z",
    companyId: null,
    status: "draft",
    createdAt: 2023-02-05T17:07:22.140Z,
    lastUsedAt: null
  }
]
{
  id: undefined,
  apiKey: undefined,
  companyId: undefined,
  status: undefined,
  createdAt: ul,
  lastUsedAt: ul
}

@AndriiSherman
Copy link
Member

@Sandy-Garrido is it better-sqlite3 driver?

@Sandy-Garrido
Copy link

Sandy-Garrido commented Feb 5, 2023

@AndriiSherman That's a good point! No, this test repository is using bun-sqlite
I have so many test repo's around at the moment for drizzle 😂

@AndriiSherman
Copy link
Member

Good, so I have fixed this one only for better-sqlite3 driver. I can do same for bun-sqlite

@AndriiSherman
Copy link
Member

Will ping you here after release

@Bulbang
Copy link
Contributor Author

Bulbang commented Feb 6, 2023

@AndriiSherman Yep, it seems to be working well now. Thanks.

@Bulbang Bulbang closed this as completed Feb 6, 2023
lightningcraft-0201 added a commit to lightningcraft-0201/React-Company that referenced this issue Oct 16, 2023
1. Issue link - drizzle-team/drizzle-orm#158
2. Add test cases for SQLite Proxy Driver
prometixX pushed a commit to prometixX/drizzle-orm-mssql that referenced this issue Jan 28, 2024
1. Issue link - drizzle-team#158
2. Add test cases for SQLite Proxy Driver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants