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

Projected local registries aren't preserved by m/form #326

Open
tekacs opened this issue Dec 30, 2020 · 2 comments
Open

Projected local registries aren't preserved by m/form #326

tekacs opened this issue Dec 30, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@tekacs
Copy link
Contributor

tekacs commented Dec 30, 2020

Perhaps there's a way to achieve this that I've simply been missing. :)

Projection in the context of registries is a place where cycling out and back in through m/form breaks down.

user> (def local [:map {:registry {:looked/up 'int?}} [:x :looked/up]])
#'user/local
user> (m/validate (mu/get local :x) 123)
true
user> (m/validate (m/form (mu/get local :x)) 123)
Execution error (ExceptionInfo) at malli.core/-fail! (core.cljc:84).
:malli.core/invalid-schema {:schema :looked/up}

When using utilities like mu/get and similar (which I use to project types), the parent's registry is seemingly preserved... until serialization.

Whether part of m/form or a secondary utility, should it be possible to perform projections like these whilst preserving the registry?

I need this functionality for my use so I've built a custom version of mu/get that:

  • constructs a composite registry using the local + passed-in registries
  • grabs the subtree using mu/get and the composite registry
  • walks the subtree using the composite registry to find any unresolved refs
  • projects the topmost local registry to just contain keys used in the subtree
  • uses update-properties assoc to put the projected registry on the new root

... but it seems like it should be possible to use the (-> ... mu/get m/form) example above without all of this messing around. :)

@ikitommi
Copy link
Member

ikitommi commented Jan 1, 2021

I see. You can get the accumulated registry with:

(-> local (mu/get :x) (m/options) :registry)

, but doesn't help much, as it's a CompoiteRegistry which has all the base schemas merged in it. I think we should:

  1. clearly separate the base registry with local (accumulated) registries, e.g. we could have a new LocalRegistry type, which contains the accumulation of all the local things
  2. helpers to get the accumulated local registry entries from a schema
  3. have an option to m/form to use the accumulated registry.
  4. figure out a default for 3: maybe the top-level m/form should always have the accumulated registry, child forms not: this way the m/form could always be compiled back into Schema, and the child forms would not contain duplicate/accumulated registry entries.

what do you think?

@ikitommi ikitommi added the enhancement New feature or request label Jan 1, 2021
@tekacs
Copy link
Contributor Author

tekacs commented Jan 5, 2021

Thank you! This makes total sense to me and seems like what I've done.

I think on (4), the natural default is the one which round-trips with highest fidelity and can be slotted back into the parent. The behaviour you're suggesting seems to match that approach. Just to make sure I understand, it sounds like the default would be:

user>
(def top
  [:map {:registry {::x 'int?}}
   ::x
   [:k
    [:schema {:registry {::y 'string?}}
     [:tuple ::x ::y]]]])
;; ...

user> (m/form top)
;; identical to `top`, so includes the top-level registry

user> (m/form (m/get top ::x))
::x

user> (m/form (m/get top :k))
[:schema {:registry {::y 'string?}}
 [:tuple ::x ::y]]

;; ^ these are useless without first being handed a registry that provides the ::x
;;   that said, it doesn't create extra noise if it's now wrapped back in [:schema {:registry ...} <THIS>]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants