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

Config option for SafeQL type literal transform #120

Closed
Eprince-hub opened this issue Apr 11, 2023 · 5 comments
Closed

Config option for SafeQL type literal transform #120

Eprince-hub opened this issue Apr 11, 2023 · 5 comments

Comments

@Eprince-hub
Copy link

Hi @Newbie012, Hope you are doing great!
Is your feature request related to a problem? Please describe.
Yes, the feature request is related to how SafeQL handles type literals. When a query returns a result set that includes literal types, SafeQL throws a Query has incorrect type annotation error. When using literal types, it is necessary to manually transform them to non-literal types using a custom type, which can be cumbersome and error-prone. This feature request is related to this problem and aims to make it easier to work with literal types in SafeQL.

This is directly related to this closed request. I know that SafeQL is working as intended according to your comment/suggestion here, but the suggestion did not work in my use case, and I had to create a custom type to workaround that.
So for the types below, SafeQl will throw the Query has incorrect type annotation error

const d = [
  { id: 1, firstName: 'b', type: 'c', accessory: 'd' }
] as const;

type G = typeof d[number];

type H = {
  readonly id: 1;
  readonly firstName: 'b';
  readonly type: 'c';
  readonly accessory: 'd';
};

export async function query() {
  return await sql<G[]>`
    SELECT * FROM animals
  `;
}

Screenshot 2023-04-07 at 18 38 15

Describe the solution you'd like
I would like SafeQL to support a configuration option to internally transform literal types to non-literal types during query linting. This would allow users to use literal types in their queries without having to manually transform them, making the process more efficient and less error-prone.

Describe alternatives you've considered

As a workaround, I created a conditional type Widen that would transform the type literals to none-literal types

export type Widen<T> = {
  [K in keyof T]: T[K] extends string
    ? string
    : T[K] extends number
    ? number
    : T[K] extends boolean
    ? boolean
    : T[K] extends Date
    ? Date
    : T[K] extends infer U | null
    ? U extends string
      ? string | null
      : U extends number
      ? number | null
      : U extends boolean
      ? boolean | null
      : U extends Date
      ? Date | null
      : U | null
    : T[K];
};

// Usage
export async function query() {
  return await sql<Widen<G>[]>`
    SELECT * FROM animals
  `;
}

Additional context
It would be great if this transformation could be done internally so the above type can be used like this with no error

export async function query() {
  return await sql<G[]>`
    SELECT * FROM animals
  `;
}

Explanation about Widen in this playground

@Newbie012
Copy link
Collaborator

In the original issue, I wrote my suggestion from my phone, so I was unable to check if I wrote it properly. This is how it should be:

import postgres from "postgres";

const sql = postgres();

type A = { id: number; firstName: string; type: string; accessory: string };
const a = [{ id: 1, firstName: 'b', type: 'c', accessory: 'd' }] as const satisfies readonly A[];

export async function query() {

  return await sql<A[]>`
    SELECT * FROM animals
  `;
}

as you can see in the playground

Now that we covered the original issue, I'll address this feature request.

While I understand how it can make life easier in some cases (prior to satisfies syntax), it would be unwise to do so.

Although, if you are certain that the database will return a type literal, you could enforce it via CREATE TYPE name AS ENUM and SafeQL will return a union of type literal (introduced in 0.0.25:

CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy');

CREATE TABLE person (
    ...,
    mood mood NOT NULL
);
sql<{ mood: "sad" | "ok" | "happy" }[]>`SELECT mood FROM person`;

Hopefully, these solutions will cover your needs.

@Eprince-hub
Copy link
Author

Eprince-hub commented Apr 13, 2023

Thanks for the quick reply.
I understand the suggested solution above, but the downside of doing something like this is the inflexibility, and the main purpose of using literal type is not to enforce the database to return some constant value like ('sad', 'ok', 'happy', ...)

CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy');

CREATE TABLE person (
    ...,
    mood mood NOT NULL
);

The insertion below will fail unless I somehow have to go and add it first to the ENUM type, which denies the flexibility/safety and SafeQL easy usage doctrine that the users would get if SafeQL somehow supports literal internally.

insert into person
  (mood)
VALUES
  ('yes');

Literal type is important and mostly very helpful when we are consigned about type safety and expressiveness. It also helps to reduce the risk of errors, which is why most developers use it for database seeding, and having a setup like the above will not achieve any flexibility/efficiency while still keeping the benefit of using the literal and SafeQL

Imagine when more data needs to be added to the database via seeding

Having a setup like below makes it more efficient and less complicated

const testData = [{ firstName: 'b', type: 'c', accessory: 'd' }] as const;

// I can add a new object to the `testData` without having to do anything else
const testData = [{ firstName: 'b', type: 'c', accessory: 'd' }, { firstName: 'y', type: 'k', accessory: 'p' }] as const;

Another very important use case for literal types is the ability to refer to known ids from another table that may change over time. For example, let's consider a quiz application where we have a Quiz table and a Topic table. Topics are added to the database via the seeder, and their ids may change over time as new topics are added or removed. By defining a literal type topicId for the id property of the Topic, we can ensure that only valid Topic ids are referenced when creating a quiz.
e.g

const topics = [
  { id: 1, name: 'History' },
  { id: 2, name: 'Math' },
  { id: 3, name: 'Science' }
] as const;

type TopicId = typeof topics[number]['id']; // his defines a literal type `TopicId` that can only take on the values of the id property of the objects in the topics array.

const quiz1: Quiz = { id: 1, topicId: 2 }; // No error because `topicId` is a valid id in the `topics` array
const quiz2: Quiz = { id: 2, topicId: 4 }; // This will result in a type error since 4 is not a valid `topicId`

This ensures that only valid Topic ids are referenced when creating a quiz, making the code more maintainable and less error-prone. If we need to add new topics via the seeder, we can do so without worrying about changing anything in the codebase that references the Topic ids.

@Newbie012
Copy link
Collaborator

I didn't recommend changing your code to ENUMs (the solution I wrote at the start of my comment is more suitable for your use case). The reason I mentioned ENUMs was due to the fact that SafeQL will return a union of literal strings on enum types, which will cause a conflict with your suggestion (more about that later in the comment).

While I understand your reasons not to use ENUMs, I'm still not sure what's wrong with the solution I wrote above using satisfies:

import postgres from "postgres";

const sql = postgres();

type A = { id: number; firstName: string; type: string; accessory: string };
const a = [{ id: 1, firstName: 'b', type: 'c', accessory: 'd' }] as const satisfies readonly A[];

export async function query() {

  return await sql<A[]>`
    SELECT * FROM animals
  `;
}

Looking at the original code feels like it's the opposite direction of how SafeQL should be used. SafeQL is the one who's responsible to determine what would be the returned type of the raw query, not the other way around. In fact, if the original code would not throw any errors, this would've been a serious bug, since we can't assume that the query returns only these literal types since there's no guarantee that it will stay like that in a different context/part of code. You could potentially add manually a new record with values that differs from the literal types that you've provided:

const d = [
  { id: 1, firstName: 'b', type: 'c', accessory: 'd' }
] as const;

type G = typeof d[number];

export async function query() {
  await sql`insert into animals (first_name, type, accessory) values ('unexpected', 'unexpected', 'unexpected')`;

  return await sql<G[]>`
    SELECT * FROM animals
  `;
}

// ReturnType<typeof query>: {id: 1, firstName: 'b', type: 'c', accessory: 'd' }[]
// Runtime: [{id: 1, firstName: 'b', type: 'c', accessory: 'd' }, {id: 2, firstName: 'unexpected', type: 'unexpected', accessory: 'unexpected' }]

While the return type of query function will be readonly {id: 1, firstName: 'b', type: 'c', accessory: 'd' }[], you will get a different result in runtime:

[
  {id: 1, firstName: 'b', type: 'c', accessory: 'd' },
  {id: 2, firstName: 'unexpected', type: 'unexpected', accessory: 'unexpected' },
  // There's no guarantees that there won't be more records in the future
]

@Eprince-hub
Copy link
Author

The solution you provided, using satisfies, is okay, but in my case, it defeats why we used the literal type in the first place. Consider the cases below

  1. My last example from my comment above where we want a certain table to accept only an Ids from another table.
// Using satisfies
type A = { id: number; firstName: string; type: string; accessory: string};
const a = [{ id: 1, firstName: 'b', type: 'c', accessory: 'd'}] as const satisfies readonly A[];

type AId = A['id']
const aId1:AId = 2 // no error because it would accept just any number

// Using `as const`
const b = [{ id: 1, firstName: 'b', type: 'c', accessory: 'd' }] as const
type B = typeof b[number]

type BId = B['id']
const bId1:BId = 2 // error because it will accept just the ids of b
  1. If the properties of the original object change, (add more or remove ), we will need to dig through our codebase and update the type everywhere it is being used
type A1 = { id: number; firstName: string; type: string; accessory: string};
const a1 = [{ id: 1, firstName: 'b', type: 'c', accessory: 'd', newProperty: ''}] as const satisfies readonly A1[]; // must be updated everywhere A1 is used

const b1 = [{ id: 1, firstName: 'b', type: 'c', accessory: 'd', newProperty: '' }] as const // Update only in one place, and B1 will have all the properties already
type B1 = typeof b1[number]
  1. The satisfies solution seems like creating an extra type that we don't necessarily need to if we are using the as const pattern.

I have the errors shown in this playground

@Newbie012
Copy link
Collaborator

You could do instead:

type A = { id: number; firstName: string; type: string; accessory: string};
const a = [{ id: 1, firstName: 'b', type: 'c', accessory: 'd'}] as const satisfies readonly A[];
type ReadonlyA = (typeof a)[number];

But the main issue is, as I mentioned, you're trying to enforce a different type than what the actual return type of the query. The best thing is to let SafeQL do its thing and use your other types separately. If you want, you could suppress SafeQL by either a comment or by doing something like:

const untypedSql = sql;

untypedSql<PlaceYourDesiredType>`...`

I'm sorry but I can't accept this feature request as it's not the intended way of using SafeQL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants