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

Support for readonly Indexed Access Type #119

Closed
Eprince-hub opened this issue Mar 21, 2023 · 5 comments
Closed

Support for readonly Indexed Access Type #119

Eprince-hub opened this issue Mar 21, 2023 · 5 comments

Comments

@Eprince-hub
Copy link

Eprince-hub commented Mar 21, 2023

Is your feature request related to a problem? Please describe.
Yes, the SafeQL plugin currently throws an error when a readonly TypeScript type is used in it. This leads to a Query has incorrect type annotation. error when using Indexed Access Types.

The setup below would throw the above error on the <A[]> because the type A is a readonly type

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

type A = typeof a[number];

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

Screenshot 2023-03-21 at 13 03 20

Screenshot 2023-03-21 at 13 04 22

Describe the solution you'd like
SafeQL should support the readonly Typescript types when it is being indexed, like in the example above. So, it should not throw an error when a readonly Indexed Access Type is used as in the example below

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

type A = typeof a[number];

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

Describe alternatives you've considered
I tried to convert the readonly type to writable again using some of these methods below, but none of the method I tried worked for me

I saw this example of creating a type that would convert it from readonly here

type DeepMutable<T> = { -readonly [P in keyof T]: DeepMutable<T[P]> };

export async function query() {
  return await sql<DeepMutable<A>>`
    SELECT * FROM animals
  `;
}

Using the Writable type from the toolbelt utility after i tried it in this simple example as shown in this sandbox and it seems to work there but doesn't work when i use it in SafeQL in my application

export async function query() {
  return await sql<Writable<A>>`
    SELECT * FROM animals
  `;
}

** None of the above method I tried worked

Additional context
Add any other context or screenshots about the feature request here.

@Newbie012
Copy link
Collaborator

Newbie012 commented Mar 22, 2023

I think it's working as intended, I'll explain.

According to your code, you expect all of the rows to be a specific row with specific values, but nothing guarantees that all of your returned rows will have the id 1.

The issue is not about readonly types, it's about incorrect type literals.

You could do something like this:

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

type A = { id: number; firstName: string; type: string; accessory: string };

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

@Eprince-hub
Copy link
Author

Thank you for your prompt response and for taking the time to explain the issue. I understand now that the problem is not related to readonly types but rather to incorrect type literals in the code.

Your suggested solution makes sense, and I will try it out to see if it works for my use case.

Thanks again for your support!

@karlhorky
Copy link
Collaborator

karlhorky commented Jun 27, 2023

Just thinking about this more, when there is literal data coming from tables (eg. tables that only contain seeded data from fixtures) it would be nice to have SafeQL correctly apply these stricter types.

One way that this could be done would be something like an (optional) custom type compatibility generic parameter as a 2nd parameter for sql to allow for comparison checks:

animals/seeder.ts

export type Animal = (typeof seededAnimals)[number];

const seededAnimals = [
  { id: 1, firstName: 'Alex', type: 'Cat', accessory: 'Collar' },
  { id: 2, firstName: 'Rita', type: 'Mouse', accessory: 'Collar' },
  { id: 3, firstName: 'Jennifer', type: 'Dog', accessory: 'Chew toy' },
] as const;

// ... more code to seed the data into the database ...

animals/queries.ts

// In another file
const animals = sql<Animal[], WidenAndMakeMutable>`SELECT * FROM animals`;
   // ^? { readonly id: 1, readonly firstName: 'Alex', readonly type: 'Cat', readonly accessory: 'Collar' } | { readonly id: 2, readonly firstName: 'Rita', readonly type: 'Mouse', readonly accessory: 'Collar' } | { readonly id: 3, readonly firstName: 'Jennifer', readonly type: 'Dog', readonly accessory: 'Chew toy' }
   // ✅ Literal data here! (type errors for comparisons with incorrect literals eg. 'Collarrrr')
   // ✅ Also, autocomplete!

Details:

  • The WidenAndMakeMutable type could be implemented by the user to be used internally in SafeQL during type checking (it would be applied to the type that the user passes in temporarily - eg. applied to Animal[])
  • SafeQL could then apply the Animal[] type to the returned array

@Newbie012
Copy link
Collaborator

While I understand that this change will make your use case possible, this approach is problematic. The assumption in your code that only seededAnimals are stored in animals is wrong.

  • There's no guarantee that there are no unexpected rows.
  • What if there's a nullable column, but your literals don't show any nullability?
Query has incorrect type annotation.
	Expected: { id: number; name: string; }
	Actual: { id: number; name: string | null; }
  • What if one of the columns of the table is enum type, but your seeded literal isn't identical to the enum's literals (partial/wrong)?

There are many other issues that can come up due to this approach. If you already have the values hardcoded, I would simply advise you to infer the type from there rather than from SafeQL.

If you really insist of doing so, you could wrap your type with a custom Widen<T> type:

const users = [
  { id: 1, name: "John" },
  { id: 2, name: "Jane" },
  { id: 3, name: "Joe" },
  { id: 3, name: null },
] as const;

type User = (typeof users)[number];

type LiteralToBase<T> = T extends string
  ? string
  : T extends number
  ? number
  : T extends boolean
  ? boolean
  : T extends null
  ? null
  : T extends undefined
  ? undefined
  : T extends bigint
  ? bigint
  : T extends symbol
  ? symbol
  : T extends object
  ? object
  : never;

type Widen<T> = {
  [K in keyof T]: T[K] extends infer U ? LiteralToBase<U> : never;
};

await sql<Widen<User>[]>`SELECT * from users`;

Again, I would advise not doing it based on what I wrote above. Instead, you could make use of satisfies:

const users = [
  { id: 1, name: "John" },
  { id: 2, name: "Jane" },
  { id: 3, name: "Joe" },
  { id: 3, name: null },
] as const satisfies readonly User[];

type UserLiteral = typeof users[number];

type User = {
  id: number;
  name: string | null;
};

await sql<User[]>`SELECT * from users`;

@karlhorky
Copy link
Collaborator

karlhorky commented Jun 27, 2023

  • There's no guarantee that there are no unexpected rows.

This may be guaranteed by the team / processes / automation

  • What if there's a nullable column, but your literals don't show any nullability?
  • What if one of the columns of the table is enum type, but your seeded literal isn't identical to the enum's literals (partial/wrong)?

These would not be an issue if there's only the seeded fixtures data in the table. And the enums would never be wrong, because the data would be seeded to the table, which would be caught with a database error if there was an issue with the data.


The suggested approach with widening the type would not work, because the type for users would then be the widened type, which is not desired.

const users = await sql<Widen<User>[]>`SELECT * from users`;
   // ^? { readonly id: number; readonly name: string; } | { readonly id: number; readonly name: string; } | { readonly id: number; readonly name: string; } | { readonly id: number; readonly name: null; }

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

3 participants