kristiandupont / kanel

Generate Typescript types from Postgres
https://kristiandupont.github.io/kanel/
MIT License
867 stars 60 forks source link

Issue with Identifier Types and Composite Foreign Keys (kanel + kysely) #571

Open dcousineau opened 3 months ago

dcousineau commented 3 months ago

TLDR Found possibly an edge case, possibly an issue on my end, involving composite foreign keys and would love guidance on next steps (fix my kanel configs? learn where in kanel this could be happening an help contribute a fix? wait for a fix?)

I have recently come across an issue where Kanel (+ Kanel-Kysely) generates problematic outputs as my schema has evolved to have more complex compound foreign keys. It's possible my configurations are bad or I'm doing something very silly.

[!IMPORTANT] I have created a code sandbox MVP reproduction to illustrate what happens. This sandbox has my exact .kanelrc.js along with the schema below.

The TLDR of the issue is when I have a moderately complex n:m type join table in situations where both sides of the join have composite primary keys. Kanel attempts to do an OR union of the "identifier" types (but for only one side of the equation) for identifier types it... doesn't actually create.

Given the following postgres schema:

CREATE TABLE organization (
  id bigserial PRIMARY KEY
);
CREATE TABLE a (
  id bigint NOT NULL,
  organization_id bigint NOT NULL REFERENCES organization(id),
  CONSTRAINT a_pkey PRIMARY KEY (id, organization_id)
);
CREATE TABLE b (
  id bigint NOT NULL,
  organization_id bigint NOT NULL REFERENCES organization(id),
  CONSTRAINT b_pkey PRIMARY KEY (id, organization_id)
);
CREATE TABLE a_b_match (
  organization_id bigint NOT NULL,
  a_id bigint NOT NULL,
  b_id bigint NOT NULL,
  CONSTRAINT a_b_match_a_fkey FOREIGN KEY (organization_id, a_id) REFERENCES a(organization_id, id),
  CONSTRAINT a_b_match_b_fkey FOREIGN KEY (organization_id, b_id) REFERENCES b(organization_id, id),
  CONSTRAINT a_b_match_pkey PRIMARY KEY (organization_id, a_id, b_id)
);

And a .kanelrc.js almost entirely based on https://github.com/kristiandupont/kanel/blob/main/example/.kanelrc.js, we get the following outputs.

[!NOTE] Specifically notice how ABMatch.ts is attempting to import BOrganizationId from B.ts and union it with OrganizationId, however B.ts doesn't actually define or export BOrganizationId

In an ideal world, Kanel would not attempt to create BOrganizationId nor would it attempt to union it with OrganizationId.

ABMatch.ts

// @generated
// This file is automatically generated by Kanel. Do not modify manually.

import { type OrganizationId } from './Organization';
import { type BOrganizationId, type BId } from './B';
import { type AId } from './A';
import { type ColumnType, type Selectable, type Insertable, type Updateable } from 'kysely';

/** Represents the table public.a_b_match */
export default interface ABMatchTable {
  organization_id: ColumnType<OrganizationId | BOrganizationId, OrganizationId | BOrganizationId, OrganizationId | BOrganizationId>;

  a_id: ColumnType<AId, AId, AId>;

  b_id: ColumnType<BId, BId, BId>;
}

export type ABMatch = Selectable<ABMatchTable>;

export type NewABMatch = Insertable<ABMatchTable>;

export type ABMatchUpdate = Updateable<ABMatchTable>;

B.ts

// @generated
// This file is automatically generated by Kanel. Do not modify manually.

import { type OrganizationId } from './Organization';
import { type ColumnType, type Selectable, type Insertable, type Updateable } from 'kysely';

/** Identifier type for public.b */
export type BId = string;

/** Represents the table public.b */
export default interface BTable {
  id: ColumnType<BId, BId, BId>;

  organization_id: ColumnType<OrganizationId, OrganizationId, OrganizationId>;
}

export type B = Selectable<BTable>;

export type NewB = Insertable<BTable>;

export type BUpdate = Updateable<BTable>;
kristiandupont commented 3 months ago

Hmm, yes, I see the problem but I admit that I am not sure where in the chain the bug lies. As with many other issues, I won't promise when I can get around to debugging it but I would like to fix this as it does appear to be a bug and not some out-of-scope feature request. If this is blocking you, a post-render hook is probably the easiest way to get around it

dcousineau commented 3 months ago

Sounds great. Except for me managing to stub my toes here kanel has been excellent to me.

I'll try the post render hook first just to bandaid the situation. If you have any examples handy (that aren't already in this repo) that would be helpful, but if not no worries!

I'll also keep you posted if I am able to debug or chase the issue down myself.

dcousineau commented 3 months ago

Something I just noticed because I bumped to 3.9.0 2 weeks ago (but hadn't had cause to re-run type generation) is I am now getting a lot of "Could not resolve circular reference" warnings (from this line: https://github.com/kristiandupont/kanel/blob/ace86e45955b2eca64b0f66a2a367b9a3cc50a30/packages/kanel/src/generators/resolveType.ts#L100) which was from #540 and the following error:

TypeError: Cannot read properties of undefined (reading 'join')
    at writeFile (./node_modules/kanel/build/writeFile.js:41:27)
    at ./node_modules/kanel/build/processDatabase.js:78:60
    at Array.forEach (<anonymous>)
    at processDatabase (./node_modules/kanel/build/processDatabase.js:78:18)
    at async main (./node_modules/kanel/build/cli/main.js:120:9

(Upon further inspection the writeFile method has a lines argument set to undefined rather than an array)

So this is a clue at least, I can't spend much more time today digging in but I'll report back when I get the chance to spend more time on it.

It appears the issue stems from a parent FKEY referencing columns in a child who are themselves also an FKEY. I'm still unsure if the composite FKEY aspect comes into play.