get-convex / convex-ents

https://labs.convex.dev/convex-ents
25 stars 5 forks source link

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

Open kyldvs opened 7 months ago

kyldvs commented 7 months ago

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 commented 7 months ago

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 ```ts queries: defineEnt({}) .edges("foos", { to: "foos", ref: true }), foos: defineEnt({}) .edge("queries", { to: "queries", field: "queryId" }) ```