malli.core/into-schema? returns true i.f.f. implements protocol interface (not protocol)

[jasonjckn]

(defn into-schema?
  "Checks if x is a IntoSchema instance"
  [x] (#?(:clj instance?, :cljs implements?) malli.core.IntoSchema x))

This function definition should call clojure.core/satisfies? not instance?

I was trying to write code like

(extend-protocol m/IntoSchema
  java.lang.Class
.........)))

And I wasn't getting the desired behaviour, because

If someone can confirm this is unintentional, i'll submit a PR.

[ikitommi]

Thanks for reporting. satisfies? is dead-slow and support for it was removed in the schema creation performance improvement epic.

Please go vote up https://clojure.atlassian.net/browse/CLJ-1814.

[ikitommi]

one option would be to first try the instance? call (should match ~100% of cases) and use satisfies? as backup. Commented on the other issue already (#961 (comment)) - malli supports Class Schemas out via registry, bit more code but explicit control.

do you still see that backup call to satisfies? would be needed here?

[jasonjckn]

one option would be to first try the instance? call (should match ~100% of cases) and use satisfies? as backup.

Thanks for the context re: CLJ-1814

My specific use case here... which is

(extend-protocol m/IntoSchema
  java.lang.Class
  (-type [^Class this] (symbol (.getName this)))

I might backtrack on my use case, i'll leave a comment @ #961

---

I'm kind of split 50/50 on this now, I still think extend-protocol m/IntoSchema is a valid use case, one expects extend-protocol to work, if not for java.lang.Class, then I'm sure I'll find utility for this down the road if not today.

I'm very sympathetic to performance, maybe if i change this code this way?

If you feel good about that... or something else, i can send a PR, if not maybe its best if I just close this and reopen later.

[jasonjckn]

I think I realized the right solution here, it's not the most elegant code but it accomplishes all the performance objectives and retains open polymorphism / extensible to any type such that into-schema? is open instead of closed — the way it is today. Just make into-schema? a protocol instead of calling satisfies? or even instance?

Thus any time you extend the protocol m/IntoSchema, you would also have to extend m/SupportsIntoSchema,

I think if you go this route, it may simplify into-schema function as well, because you'll be able to do (extend-protocol m/IntoSchema IPersistentVector instead of handling it as a special case .... but if you don't care for this specific part, no worries.

shall I send a PR?

P.S. I wonder why clojure core team didn't implement satisfies? like this from the get-go.

[jasonjckn]

I guess i could have also supported java.lang.Class by defining a protocol 'RegistryKeyLookup', and extend it for java.lang.Class , and java.lang.Object fallback to do an actual Registry lookup.

but nonetheless i feel like the above is still a nice design... especially if you extend protocol for IPersistentVector,
it'll perform about the same, but a bit more modular,

would become basically just

could even perform a tiny bit faster