lambdaisland / facai

Factories for fun and profit. 恭喜發財!
Mozilla Public License 2.0
45 stars 1 forks source link

[BREAKING] Automatically defer when invoking a factory #5

Closed plexus closed 1 year ago

plexus commented 2 years ago

This is a breaking change, we are still in alpha after all, and I think this is worth it. I'll do my best to explain my reasoning.

Up to now invoking a factory (calling it as a function) would be equivalent to build-val, that is: it returns an instantiated value based on the factory template, and any options you pass in like :with, :traits, :rules.

But, if you called a factory inside another defactory, we would defer the build process, we simply captured that this is the factory and these are the options, and when the actual build happens we apply them.

(f/defactory foo
  {:name "Arne"})

(foo) ;;=> {:name "Arne"}

(f/defactory bar
  {:foo (foo {:with {:name "Ben"}})})

(:facai.factory/template bar)
;;=>
{:foo (->DeferredBuild foo {:with {:name "Ben"}})}

This way you can then do things like. In other words we apply late binding, so that you can still override and influence things when building the top-level thing. It's also vital when working with the database layers, only entities that get created as part of the process get inserted. If it's already instantiated to a plain clojure value then the db layer doesn't know what to do with that.

(f/build-val bar {:rules {[foo :name] "Paul"})

The problem that this creates is that you can't do this

(def foo-with-ben
  (foo {:with {:name "Ben"}}))

(f/defactory bar
  {:foo foo-with-ben})
;; or during building
(f/build-val bar {:with {:foo foo-with-ben}})

So even though this is a straightforward refactoring of the earlier code, it breaks. I worked around this by introducing some explicit helpers to create these deferred-build instances, but I don't really want people to have to think about when they need to use them.

The thing is also that when using Facai I expect people to mostly be using it via a database layer, so you're always calling some kind of create! function explicitly, why not repurpose the direct-invocation API to provide a cleaner syntax, by always returning a deferred?

We can also use this to fix another issue, overriding plain values is extremely common, and always having to do {:with ...} kind of sucks. So the idea is that if you pass a map, we treat these as overrides. If you use keyword args, we treat them as options.

(foo {:name "Arne"}) ;; overrides, :with is implicit
(foo :with {:name "Arne"} :traits ["authenticated"]) ;; options

There might be another (and somewhat bigger) breaking change that falls out of this, in that we might want to apply this same syntax to build/build-val/create etc, for symmetry

(f/build-val foo {:name "Arne"})
(f/build-val foo :traits ["cool])
plexus commented 1 year ago

One point in particular that I would like input on is the syntax, I'd like to provide a shorter alternative to always having to enter (my-factory {:with ...}), but not sure which of these is best

;; Option 1 (❤️): keyword-args for options, map for override
(f/build-val (user :with {:user/name "Mellissa Schimmel"}))
(f/build-val (user {:user/name "Mellissa Schimmel"}))

;; Option 2 (🎉): map for explicit options, keyword-args for override
(f/build-val (user {:with {:user/name "Mellissa Schimmel"}
                    :rules {}
                    :traits []}))
(f/build-val (user :user/name "Mellissa Schimmel"))

;; Option 3: (🚀) allow both map or keyword-args, but always specifiy option keys
(f/build-val (user {:with {:user/name "Mellissa Schimmel"}}))
(f/build-val (user :with {:user/name "Mellissa Schimmel"}))
plexus commented 1 year ago

Even though this isn't exactly fully baked I'm merging this now, since we're proceeding with the rename to Harvest, and this will be a pain to merge afterwards.

plexus commented 1 year ago

I've removed this commit from main here, instead deciding to keep facai stable and not have any more breaking changes. Since we're renaming anyway we can do new development under Harvest, which does have these commits.