freckle / graphula

A simple interface for generating persistent data and linking its dependencies
MIT License
45 stars 11 forks source link

Cannot use with entities not `SafeToInsert` #70

Closed pbrisbin closed 9 months ago

pbrisbin commented 1 year ago

As of lts-21, the version of persistent present needs SafeToInsert to use insert. This serves as witness that the entity in fact uses an auto-increment key and so is "safe to insert [without explicit key]". Entities where this is not true must use insertKey and supply the key. insertKey, of course, does not enforce SafeToInsert. So safe.

In Graphula, we have only the single insert member in the MonadGraphulaFrontend typeclass. And it always requires (our compatibility type for) SafeToInsert. This is because it takes a Maybe Key argument and uses Persist.insert on Nothing (or Persist.insertKey on Just).

Up until lts-21, this constraint was a no-op and we in fact created cases where we use Graphula on entities that aren't safe to insert. Since we always supplied a Just key to our insert for these cases (based on KeySource), it all worked. But once that constraint becomes actually-enforced, they no longer type check.

At this point, in order to depend on newer persistent and keep up with the latest LTS, we think we need to expand the type-class to include insertKey, which will be used by nodeKeyed (not node), and then we can migrate these non-compiling use-cases.

This calls into question the overall design of insert, with its Maybe argument, node and KeySource, etc -- as we seem to be diverging with how persistent is tackling a similar design space. If it turns out we can't avoid a major version bump, we might consider taking larger liberties here.

However, if we can do this as a minor version bump that would be preferred. It could be possible if we give insertKey some default. But we have to do that in a way that doesn't keep the SafeToInsert constraint on nodeKeyed when used. (For example, insertKey = insert . Just won't work.) :thinking:

/cc @tomcarste

pbrisbin commented 1 year ago

And note that this is not a problem compiling graphula itself, it only breaks with a particular use-case. We might want to add a test case for the now-broken way we use KeySource on none-safe-to-insert entities.

chris-martin commented 9 months ago

Done in #73