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

Improved error messaging for conflicting link defs in schema.ts #296

Merged
merged 4 commits into from
Oct 1, 2024

Conversation

markyfyi
Copy link
Contributor

@markyfyi markyfyi commented Sep 30, 2024

The schema planner doesn't account for a sneaky invariant: what if the user has a link def, but in the opposite direction of one in the database? As in, the forward and reverse identities are swapped. Right now, we catch this, but throw a misleading "check your schema file for dupes" error. In this change, I add some code to check for this case, and throw a (hopefully) more informative error.

To repro - go to sandbox/cli-nodejs/instant.schema.ts and swap the forward and reverse props of one of the links.

Copy link

View Vercel preview at instant-www-js-backwards-link-error-jsv.vercel.app.

Marky @ Instant added 2 commits September 30, 2024 10:25
@markyfyi markyfyi changed the title Improved error messaging for conflicting link defs in schema-as-code Improved error messaging for conflicting link defs in schema.ts Sep 30, 2024
Copy link
Contributor

@nezaj nezaj left a comment

Choose a reason for hiding this comment

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

LGTM w/ one nit.

I like the informative error, Stripe goes pretty with some of their error messages and I think it's a really nice user experience

Copy link
Contributor

@stopachka stopachka left a comment

Choose a reason for hiding this comment

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

LGTM!

@markyfyi markyfyi merged commit 7a24bbf into main Oct 1, 2024
22 checks passed
@markyfyi markyfyi deleted the backwards-link-error branch October 1, 2024 17:57
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.

3 participants