Skip to content

Comments

dynamically infer many to many primary key types#34

Closed
l0g1x wants to merge 1 commit intoPassionfroot:mainfrom
l0g1x:fix-many-to-many
Closed

dynamically infer many to many primary key types#34
l0g1x wants to merge 1 commit intoPassionfroot:mainfrom
l0g1x:fix-many-to-many

Conversation

@l0g1x
Copy link

@l0g1x l0g1x commented Apr 26, 2025

previously, many-to-many primary key types where being hardcoded to string()

Copilot AI review requested due to automatic review settings April 26, 2025 03:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR dynamically infers many-to-many primary key types instead of hardcoding them to string(), and updates the many-to-many join table configuration accordingly.

  • Infers primary key types using mapPrismaTypeToZero.
  • Introduces explicit checks for primary key existence with clear error messages.
  • Refactors relationship definitions to use locally stored primary key fields.
Files not reviewed (1)
  • src/tests/snapshots/generator.test.ts.snap: Language not supported
Comments suppressed due to low confidence (2)

src/mappers/schemaMapper.ts:98

  • [nitpick] Since modelA_pkField is already verified to exist earlier, consider simplifying to 'destField: [modelA_pkField.name]' for clarity.
destField: modelA_pkField.name ? [modelA_pkField.name] : [],

src/mappers/schemaMapper.ts:104

  • [nitpick] Since modelB_pkField is already verified to exist earlier, consider simplifying to 'destField: [modelB_pkField.name]' for clarity.
destField: modelB_pkField.name ? [modelB_pkField.name] : [],

@elledienne
Copy link
Contributor

Thanks for the PR @l0g1x! I am going to merge #36, which supersedes your PR (adds tests and a couple of other minor things), but appreciate you looking into it and providing a fix

@elledienne elledienne closed this Apr 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants