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

Usability: Table name pluralization in edge() vs edges() is confusing and hard to use #21

Open
kyldvs opened this issue Mar 16, 2024 · 1 comment
Labels
enhancement New feature or request needs feedback Looking for justification via real world use cases

Comments

@kyldvs
Copy link

kyldvs commented Mar 16, 2024

I don't think it's very usable to make the table name change pluralization when adding an edge() vs adding an edges(). It seems okay in simple cases:

configs: defineEnt({})
  .edges("foos", { ref: true }),

foos: defineEnt({})
  .edge("config"),

But starts to encourage weird names when you are using tables with non-standard pluralizations:

queries: defineEnt({})
  .edges("foos", { ref: true }),

foos: defineEnt({})
  .edge("querie"), // Wrong singular form, but builds. Using the correct "query" doesn't build.

Since you already have edge() vs edges() to distinguish 1 and many, I think it's better to always use the exact table name instead of using string manipulation to drop the "s" in the edge() case:

queries: defineEnt({})
  .edges("foos", { ref: true }),

foos: defineEnt({})
  .edge("queries"), // The simpler thing is more ergonomic and makes more sense. Right now this doesn't build.
@xixixao xixixao added enhancement New feature or request needs feedback Looking for justification via real world use cases labels Mar 19, 2024
@xixixao
Copy link
Owner

xixixao commented Mar 19, 2024

The issue is that this way the inferred field names would have to be queriesId, which goes against the typical convention of userId or authorId. But perhaps this is a fine tradeoff.

The current workaround is:

queries: defineEnt({})
  .edges("foos", { ref: true }),

foos: defineEnt({})
  .edge("query", { to: "queries", field: "queryId" })

I did call out this issue in the docs now:

this is helpful when the simple pluralization doesn't work, like edge "category" and table "categories"

Some people also argued that these naming inferences are bad altogether and we should always explicitly specify all the names in the config (which you are welcome to do).

You can also do this if you want to stick with plurals
queries: defineEnt({})
  .edges("foos", { to: "foos", ref: true }),

foos: defineEnt({})
  .edge("queries", { to: "queries", field: "queryId" })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs feedback Looking for justification via real world use cases
Projects
None yet
Development

No branches or pull requests

2 participants