duct-framework / core

The core library of the Duct framework
76 stars 21 forks source link

spike on treating include as a module #10

Closed flyingmachine closed 6 years ago

flyingmachine commented 6 years ago

This is a rough pass at rewriting include so that it's a module.

One major, breaking change is that it requires config.edn files to contain vectors instead of maps. That allows the includes to be loaded before the rest of the config, without having to treat :duct.core/include as a special key.

flyingmachine commented 6 years ago

I didn't bother with getting tests to pass because I wanted your feedback on the approach before putting more time into it :)

flyingmachine commented 6 years ago

Here's some test output that the resulting config is basically what you want. The only difference is that :duct.core/include doesn't match what's expected, but I don't think that will matter for actual use:

Fail in including
includes
expected: (=
 (core/prep config)
 {:duct.core/include ["duct/included" "duct/config"],
  :duct.core-test/a {:x 1},
  :duct.core-test/b {:x 2, :y 2},
  :duct.core-test/c {:x 3}})
  actual: (not
 (=
  {:duct.core/include
   (#:duct.core-test{:b {:y 2}}
    {:duct.core/include (#:duct.core-test{:b {:y 2}}),
     :duct.core-test/a {:x 1},
     :duct.core-test/b {:y 2}}),
   :duct.core-test/b {:x 2, :y 2},
   :duct.core-test/c {:x 3},
   :duct.core-test/a {:x 1}}
  {:duct.core/include ["duct/included" "duct/config"],
   :duct.core-test/a {:x 1},
   :duct.core-test/b {:x 2, :y 2},
   :duct.core-test/c {:x 3}}))
weavejester commented 6 years ago

Thanks for writing this, but I had a slightly different approach in mind.

The current design I'm playing around is to change the configuration so that it is only modules. The configuration is initiated once and then traversed in dependency order to get an ordered collection of functions. These functions are applied in order to an empty map, producing another Integrant configuration. This second configuration is then initiated, and the system runs as normal.

Under this system, a Duct configuration that currently looks like:

{:duct.core/project-ns  foo
 :duct.core/environment :production

 :duct.module/logging {}
 :duct.module.web/api {}
 :duct.module/ataraxy {"/" :index}

 :foo.handler/index {}}

Would instead look like:

{:duct.module/logging {}
 :duct.module.web/api {}
 :duct.module/ataraxy {"/" :index}

 :duct.profile/base
 {:duct.core/project-ns  foo
  :duct.core/environment :production

  :foo.handler/index {}}}

As in your PR, a profile would just be a type of module that merges its content into the configuration its passed.

Ordering would be handled through prep-key. When we prep the module configuration, default references can be added that define a dependency order. Here's the above configuration, after prep has been applied to it:

{:duct.module/logging {:requires [#ig/ref :duct.profile/base]}
 :duct.module.web/api {:requires [#ig/ref :duct.module/logging]}
 :duct.module/ataraxy {:requires [#ig/ref :duct.module/web], "/" :index}

 :duct.profile/base
 {:duct.core/project-ns  foo
  :duct.core/environment :production

  :foo.handler/index {}}}

The advantage of this approach is that it produces a very clear execution order:

(-> config ig/prep ig/init duct/build-config ig/prep ig/init)

(Note that there are no side effects at all except in the initial reading of the configuration, and the final ig/init.)

This removes a number of inconsistencies from the current way of doing things:

  1. No special keys like :duct.core/include are needed
  2. Modules are not treated specially
  3. Module and include order handled by reference dependencies
  4. References between modules work as expected
flyingmachine commented 6 years ago

Wow, thanks for taking the time to explain all that! This looks wonderful and I can't wait to use it :) I really like how you handle the dependency order. So clean! So elegant!