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

Arbitrary string ids #8645

Merged
merged 6 commits into from Jun 23, 2023
Merged

Arbitrary string ids #8645

merged 6 commits into from Jun 23, 2023

Conversation

molomby
Copy link
Member

@molomby molomby commented Jun 21, 2023

In alignment with general best practice and the GraphQL spec, Keystone likes IDs to be opaque and arbitrary – without any meaning or reference to the data they represent. But Keystone goes beyond these recommendations, limiting devs to 3 specific types of ID: auto incs, uuids, and cuids.

This was a design decision intended to lead devs away from "meaningful" ids that often cause problem as a project grows. I've argued against custom IDs myself in the past, but the fact is, it's perfectly reasonable for devs to want more control over their IDs and it's not Keystone's place to stand in the way.

For example, the current implementation prevents devs from using the newer version of cuids (cuid2), ulids, Nano ID or any number of other suitable ID formats. For a system that prides itself on the "escape hatches" it offers devs, intentionally blocking these options is counterproductive.

Fortunately, it's easy to fix. I've done so here by introducing a kind: 'string' option for the idField config. The new kind falls back to cuid values by default but allows the user to provide any string value they want in a resolveInput hook. The id field filter code has been updated to allow any string to be queried. I've added a simple example project to demo the functionality.

As above, this change is intended only to allow custom ID formats, not wholly custom ID values. As such it doesn't add the id field to the create or update GraphQL types for a list, so it doesn't address #4824 / #6034.

It's also worth noting that the exact functionality of these ids will differ by DB provider due to case-sensitivity. This is a complexity developers will be opting into by overriding the default id behaviour.

@changeset-bot
Copy link

changeset-bot bot commented Jun 21, 2023

⚠️ No Changeset found

Latest commit: a30bbb1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 21, 2023

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.

Latest deployment of this branch, based on commit a30bbb1:

Sandbox Source
@keystone-6/sandbox Configuration

@gautamsi
Copy link
Member

I can not wait to get my hands on this, I was going to write custom prisma extension/patch to do this for me

@dcousens
Copy link
Member

dcousens commented Jun 23, 2023

Thanks @molomby!
This should help users write a temporary work-around to resolve GHSA-5fp6-4xw3-xqq3 if desired

@dcousens dcousens merged commit 5983cd4 into main Jun 23, 2023
58 checks passed
@dcousens dcousens deleted the molomby/custom-ids branch June 23, 2023 01:14
@gautamsi
Copy link
Member

👏
next stop Relationship reference field, prisma supports it so that I do not have to always store id for reference field, it can be email which is unique in author field.

I will open new issue or create PR when i have time

@gautamsi
Copy link
Member

gautamsi commented Jul 1, 2023

@dcousens @molomby is this not being released? see no changeset or any Version Package PR

@dcousens
Copy link
Member

dcousens commented Jul 1, 2023

@gautamsi I need to resolve problems in #8648, but my family and I have come down with COVID so I'm out-of-action for now

@gautamsi
Copy link
Member

gautamsi commented Jul 1, 2023

Take care, will wait.

dcousens added a commit that referenced this pull request Jul 10, 2023
… (#8648)

Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
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 this pull request may close these issues.

None yet

3 participants