Shorter many to many relation names

[emmatown]

Closes #6812

This makes two-sided many to many relationships only include one of the sides in the relation table (which Prisma will then include in the table name with an _ in front). I've also removed _many from the end of one-sided many relationships for consistency.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/keystonejs/keystone-next-docs/DoBsBFAC3cLzTVEizRryvG1baDRY
✅ Preview: https://keystone-next-docs-git-shorter-many-to-many-r-886fc3-keystonejs.vercel.app

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[dcousens]

Rebased

[dcousens]

I think we have a few paths with this:

• Recommending a migration strategy in the release notes ✅
• Adding a temporary option to opt-in to the previous behaviour of config.experimental.useLegacyRelationNames, or
• Adding a temporary option to opt-in to the new behaviour of config.experimental.useShortRelationNames

The useShortRelationNames option could help a user who does a blind update, as it removes any automated migration in the default case. The automatic DROP TABLE if we opt for useLegacyRelationNames is worrying if users aren't being careful and reading the generated migration.

The downside, is we are simply kicking that can down the road.

Walking through the useMigrations: true flow with this and the useMigrations: false, I am happy with useLegacyRelationNames in that Prisma shows enough warnings that any code review and or any attempt at running this in a local development environment warns the user about a destructive action.

I am strongly against the useMigrations flag generally being opt-in instead of opt-out. I think that needs improvement.

[dcousens]

@mitchellhamilton I have defaulted this to no, this should help prevent an unintentional DROP.

[molomby]

If you were building the DB yourself, most people would name these adjacency tables for the two tables they related. Eg, given User.reviewedPosts <-> Post.reviewedByUsers you'd name the table users_posts, User_Post or something like that. Obviously the problem with this generally is when you have multiple many-to-many relationships, like, say you also have User.contributedToPosts <-> Post.contributors. In that case (again, assuming you were doing this "by hand") you'd just pick sensible names for the tables, eg: maybe PostReviews for the former and PostContributions for the latter. Problem solved.

Having multiple many-to-many relationships between two lists is going to be pretty rare for most systems but of course, we still need to solve for it in the general case. My "perfect world" solution would look something like this:

• By default, adjacency tables are named for ${LEFT_LIST}_${RIGHT_LIST}, as above
• We add a config option to relationship fields that lets app builders specify a alternate "relationship key" if they want
• If you have multiple many-to-many relationships between two lists with no relationship keys Keystone throws an error on startup, directing devs to specify a value for one of the colliding relationships
• I suppose we'd also need to validate the dev hasn't set conflicting keys/names on both sides of the same relationship, etc.

This is more work that some other solutions we could use but the upsides are..

• It follows our general principal that the automatically generated stuff should, ideally, be "what you'd build yourself if you had time"
• It give us another option for people when upgrading to this release, eg. "if you don't want your DB structure to change, update the config for all your existing many-to-many relationships and specify a relationship key matching your the current table names" (It might be worth it just for this, no?)
• Letting app builders specify a "relationship key"/table name to use will sometimes be handy even if there's no naming conflict. Eg. to align with an existing DB structure or, really, by anyone who care about the details of their DB structure

[dcousens]

I think users_reviewed_posts (approximately what this implements) neatly accounts for the multiple many-to-many scenario without issue and meets the canonical expectation.

The problem is we have users_reviewedPosts_posts_reviewedByUsers now, and that's triggering PostgreSQL inbuilt limits quickly.

The primary concern for me is that by automatically generating migrations [with this change], without enforcing user consideration, could be an unnecessary foot gun. Maybe I'm too conservative.

[molomby]

I think users_reviewed_posts (approximately what this implements) neatly accounts for the multiple many-to-many scenario without issue and meets the canonical expectation.

Yeah the current solution is fine – I like that it prevents collisions but I don't love the way it arbitrarily picks the name (ie. depending on which list is left and which is right). Like, for User.reviewedPosts <-> Post.reviewedByUsers my table's either going to be User_reviewedPosts or Post_reviewedByUsers, both of which are ok but, as a developer, I don't know which I'm going to get.

Also, if you're going to give people a way to effect the name of these tables (ie. useLegacyManyRelationNames) why not just let them pick it? Eg. instead of...

const relationName = `${leftRel.listKey}_${leftRel.fieldPath}${
    useLegacyManyRelationNames ? `_${rightRel.listKey}_${rightRel.fieldPath}` : ''
}`;

Why not...

const relationName = (leftRel.name || rightRel.name) || `${leftRel.listKey}_${rightRel.listKey}`;

The primary concern for me is that by automatically generating migrations [with this change], without enforcing user consideration, could be an unnecessary foot gun. Maybe I'm too conservative.

Yeah, this is the more important problem really, I just don't have any helpful opinion on it.