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

Regression: Some JsonbAgg types returning unknown #353

Open
karlhorky opened this issue Dec 30, 2024 · 5 comments · Fixed by #354
Open

Regression: Some JsonbAgg types returning unknown #353

karlhorky opened this issue Dec 30, 2024 · 5 comments · Fixed by #354

Comments

@karlhorky
Copy link
Collaborator

karlhorky commented Dec 30, 2024

Describe the bug

With the overrides.types.jsonb configuration:

overrides: {
  types: {
    jsonb: 'JsonbAgg',
  },
},

One of my queries which was previously yielding the type JsonbAgg now yields unknown | null:

type JsonbAgg = { is_guest: boolean; };

type User = { lecturer: JsonbAgg | null };

// 💥 Query has incorrect type annotation.
//	Expected: { lecturer: JsonbAgg | null; }
//	Actual: { lecturer: unknown | null; }[]
await sql<User[]>`
  SELECT
    CASE
      WHEN lecturers.id IS NOT NULL THEN (
        jsonb_build_object(
          'is_guest',
          lecturer_users.email NOT LIKE '%@upleveled.io'
        )
      )
      ELSE NULL
    END AS lecturer
  FROM
    lecturers
    INNER JOIN users lecturer_users ON lecturers.user_id = lecturer_users.id
`;

Unfortunately, unknown | null cannot be used in a type alias (possibly because TypeScript evaluates that to be unknown):

type User = { lecturer: unknown | null };

// 💥 Query has incorrect type annotation.
//	Expected: { lecturer: unknown; }
//	Actual: { lecturer: unknown | null; }[]
await sql<User[]>`
  SELECT
    CASE
      WHEN lecturers.id IS NOT NULL THEN (
        jsonb_build_object(
          'is_guest',
          lecturer_users.email NOT LIKE '%@upleveled.io'
        )
      )
      ELSE NULL
    END AS lecturer
  FROM
    lecturers
    INNER JOIN users lecturer_users ON lecturers.user_id = lecturer_users.id
`;

Workaround:

Use the unknown | null in an inline generic type argument:

// ✅
await sql<{ lecturer: unknown | null }[]>`
  SELECT
    CASE
      WHEN lecturers.id IS NOT NULL THEN (
        jsonb_build_object(
          'is_guest',
          lecturer_users.email NOT LIKE '%@upleveled.io'
        )
      )
      ELSE NULL
    END AS lecturer
  FROM
    lecturers
    INNER JOIN users lecturer_users ON lecturers.user_id = lecturer_users.id
`;

So 2 problems here:

  1. The JsonbAgg type is no longer returned here
  2. unknown | null probably should never be inferred by SafeQL, because it's not compatible with TypeScript unknown | null inside of a type alias

To Reproduce
Steps to reproduce the behavior: See above

Expected behavior

lecturer receives a JsonbAgg type (or a specific type based on jsonb_build_object inference), as SafeQL did before the versions released for literal inference:

Screenshots

--

Desktop (please complete the following information):

  • OS: macOS Sequoia 15.2 (24C101)
  • PostgreSQL version 14.13
  • Version 3.6.0

Additional context

Problem seems to have been introduced in the versions released for literal inference:

@Newbie012
Copy link
Collaborator

The types passed in overrides.types are treated as "reserved" types. This means that whenever SafeQL compares expected and actual, it recursively flattens the types as long as they are not "reserved." This decision was made in #219 due to #218.

The above-mentioned issue should, of course, be addressed, but the proposed solution will be slightly different from what you might expect.

Instead of:

type JsonbAgg = { is_guest: boolean; };
type User = { lecturer: JsonbAgg | null };

await sql<User[]>`
  SELECT ...
`;

You'll need to use a different name than JsonbAgg or allow SafeQL to automatically infer it:

type Lecturer = { is_guest: boolean; };
type User = { lecturer: Lecturer | null };

await sql<User[]>`
  SELECT ...
`;

@karlhorky
Copy link
Collaborator Author

karlhorky commented Jan 3, 2025

Great, inference is even better! Confirmed that #354 (@ts-safeql/eslint-plugin@3.6.1) fixed it:

@karlhorky
Copy link
Collaborator Author

@Newbie012 reopening this one, it appears that it is inferring unknown again:

@ts-safeql/eslint-plugin@3.6.1

type User = {
  lecturer: {
    is_guest: boolean;
  } | null;
};

// ✅
await sql<User[]>`
  SELECT
    CASE
      WHEN lecturers.id IS NOT NULL THEN (
        jsonb_build_object(
          'is_guest',
          lecturer_users.email NOT LIKE '%@upleveled.io'
        )
      )
      ELSE NULL
    END AS lecturer
  FROM
    lecturers
    INNER JOIN users lecturer_users ON lecturers.user_id = lecturer_users.id
`;

@ts-safeql/eslint-plugin@3.6.2

type User = {
  lecturer: {
    is_guest: boolean;
  } | null;
};

// 💥 Query has incorrect type annotation.
//	Expected: { lecturer: null | { is_guest: boolean } }[]
//	  Actual: { lecturer: unknown | null }[]
await sql<User[]>`
  SELECT
    CASE
      WHEN lecturers.id IS NOT NULL THEN (
        jsonb_build_object(
          'is_guest',
          lecturer_users.email NOT LIKE '%@upleveled.io'
        )
      )
      ELSE NULL
    END AS lecturer
  FROM
    lecturers
    INNER JOIN users lecturer_users ON lecturers.user_id = lecturer_users.id
`;

@karlhorky karlhorky reopened this Jan 7, 2025
@karlhorky karlhorky changed the title Some JsonbAgg types returning unknown Regression: Some JsonbAgg types returning unknown Jan 7, 2025
@Newbie012
Copy link
Collaborator

Can you check if 3.6.3 resolves it? I wasn't quite able to reproduce it

@karlhorky
Copy link
Collaborator Author

I see it still reproducing with @ts-safeql/eslint-plugin@3.6.4, yeah:

Screenshot 2025-01-08 at 23 19 44

type User = {
  lecturer: {
    is_guest: boolean;
  } | null;
};

// 💥 Query has incorrect type annotation.
//	Expected: { lecturer: null | { is_guest: boolean } }[]
//	  Actual: { lecturer: unknown | null }[]
await sql<User[]>`
  SELECT
    CASE
      WHEN lecturers.id IS NOT NULL THEN (
        jsonb_build_object(
          'is_guest',
          lecturer_users.email NOT LIKE '%@upleveled.io'
        )
      )
      ELSE NULL
    END AS lecturer
  FROM
    lecturers
    INNER JOIN users lecturer_users ON lecturers.user_id = lecturer_users.id
`;

Here's the table definitions:

CREATE TABLE users (
  id integer PRIMARY KEY GENERATED ALWAYS AS IDENTITY
);

CREATE TABLE lecturers (
  id integer PRIMARY KEY GENERATED ALWAYS AS IDENTITY,
  email varchar(80) UNIQUE,
  user_id integer UNIQUE NOT NULL REFERENCES users (id)
);

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

Successfully merging a pull request may close this issue.

2 participants