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

PrimaryColumns other than 'id' are not supported in codegen #259

Open
jeffrey-computers opened this issue Mar 21, 2024 · 1 comment
Open

Comments

@jeffrey-computers
Copy link

jeffrey-computers commented Mar 21, 2024

Describe the bug
TypeORM @PrimaryColumn supports fields with an arbitrary name, but Squid's codegen only supports a PrimaryColumn named 'id'. If an 'id' value is not included in the schema, then it is added automatically.

This means code comments are required to explain what the entity's id value is, rather than semantic field names. As is evidenced in your own example project!

To Reproduce
Create a schema.graphql file like:

type TestEntity @entity {
  someUniqueIdentifier: ID!
  foo: boolean;
}

Codegen automatically creates a @PrimaryColumn_() of id. The intended primary column is someUniqueIdentifier

Expected behavior

  • Schema entity fields typed as ID! should be used for TypeORM's PrimaryColumn instead of id.
  • No id field should be created unless an ID! is missing.

i.e. the above example
Should yield:

@Entity_()
export class TestEntity {
    constructor(props?: Partial<TestEntity>) {
        Object.assign(this, props)
    }

    @PrimaryColumn_()
    someUniqueIdentifier!: string

    @Column_("bool", {nullable: true})
    foo!: boolean | undefined | null
}
@rmcmk
Copy link

rmcmk commented Jul 11, 2024

I agree. Primary keys should not be restricted in this manner. Additionally, supporting composite primary keys would allow for more flexible indexing paradigms.

However, this may not be a trivial change. The code generator adheres to the OpenCRUD spec, which currently only supports primary keys of type ID (essentially a string) and does not support composite primary keys (maybe it does, am not completely familiar with the spec)

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