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

Add :gen/resolve property #1043

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jgdavey
Copy link

@jgdavey jgdavey commented Apr 19, 2024

This works much like :gen/gen, but will first attempt to resolve the fully-qualified symbol.

This separation means that malli schemas are free to carry around this extra data about where generators live, without having to have them loaded at runtime. Instead, generators declared in this way are dynamically loaded when used.

This works much like `:gen/gen`, but will first attempt to resolve the
fully-qualified symbol. This separation means that malli schemas are
free to carry around this extra data about where generators live,
without having to have them loaded at runtime. Instead, generators
declared in this way are dynamically loaded when used.
@ikitommi
Copy link
Member

Thanks for the PR. Quick comments:

  1. CLJS tests fail currently (resolve is a macro)
  2. this enables running arbitrary code, so there should be a mechanism to disable this (if one reads serialised schemas from untrusted sources) - like there is with SCI.
[:int {:gen/resolve 'mount.core/stop}]

@jgdavey
Copy link
Author

jgdavey commented Apr 22, 2024

Thanks for the feedback!

CLJS tests fail currently (resolve is a macro)

I'm curious if there's a pattern that makes more sense for lazy-loading that would be regardless of platform.

this enables running arbitrary code, so there should be a mechanism to disable this (if one reads serialised schemas from untrusted sources) - like there is with SCI.

That's a good idea. I might look into sci or dynaload or somesuch for this kind of dynamic loading.

Taking a step back, is this idea useful? And if so, is this the way that it ought to be implemented? My desire is for a serializable schema (dependency-free) that can still point at more sophisticated generators that may or may not be available at runtime. My use-case is generators that do exist on the classpath in production, but are available to tests.

@frenchy64
Copy link
Collaborator

frenchy64 commented May 16, 2024

IMO this is useful. Perhaps a simpler cross-platform implementation would be m/eval :gen/gen if it is not a generator, and then have users of the schema manually load the generator namespaces when needed.

(ns spec-ns
(def schema [:map {:gen/gen `(gen-ns/my-generator)} ...])

(ns gen-ns)
(defn my-generator [] ...)

(ns test-ns
  (:require spec-ns gen-ns))
(m/generate spec-ns/schema)

It would also be nice if :gen/gen could be a fn? that gets auto-called to retrieve the generator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants