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

schema.core/protocol is incompatible with metadata-based protocol implementation #424

Open
vemv opened this issue Jul 27, 2020 · 3 comments

Comments

@vemv
Copy link

vemv commented Jul 27, 2020

s/protocol uses clojure.core/satisfies?:

{:proto-pred #(satisfies? ~p %)

In turn, satisfies? does not honor metadata-based protocol implementation: https://dev.clojure.org/jira/browse/CLJ-2426

metadata-based protocol implementation is a fine tool that can solve a variety of problems. Having s/protocol fail on it is very inconvenient.

You may find inspiration for a drop-in replacement for satisfies? here.

Thanks - V

@w01fe
Copy link
Member

w01fe commented Jul 28, 2020

Thanks for the pointer. That said, I think my opinion is that it's not this library's job to work around bugs in Clojure (especially since users are free to write their own protocol schema). For example, I would be worried that any implementation we add could be broken by a new Clojure(Script) release. Is there a reason I'm not seeing why the workaround would need to live here?

@vemv
Copy link
Author

vemv commented Jul 28, 2020

Hi @w01fe , thanks for the reply!

especially since users are free to write their own protocol schema

This is not practical, since there are 3rd party libraries written in Schema. I cannot control whether they use s/protocol or my/protocol.

not this library's job to work around bugs in Clojure

That's one way to conceptualize it. Another is that s/protocol currently uses satisfies? as an implementation detail. Coupling protocol and satisfies? behavior 1:1 is a choice; one is free to base protocol in satisfies? and extra logic.

For example, I would be worried that any implementation we add could be broken by a new Clojure(Script) release.

I have made the effort of trying to imagine such an scenario and fail to see it possible. The worst thing that can possibly happen is that the extra check becomes redundant because in a future clojure.core/satisfies? would be fixed.

There's the possibility that https://dev.clojure.org/jira/browse/CLJ-2426 gets closed in the opposite direction (i.e. "satisfies? will never work on metadata-based extensions"), in that case nothing would change for existing Schema users, and one would remain able to use metadata-based extensions (at the cost of differing opinions, luckily protocol is not called satisfies? or such)

@w01fe
Copy link
Member

w01fe commented Jul 29, 2020

That's one way to conceptualize it. Another is that s/protocol currently uses satisfies? as an implementation detail. Coupling protocol and satisfies? behavior 1:1 is a choice; one is free to base protocol in satisfies? and extra logic.

https://clojure.org/reference/protocols says "Protocols are fully reified and support reflective capabilities via extends? , extenders , and satisfies?." Is there documentation of another public interface that can be used to detect satisfaction?

I have made the effort of trying to imagine such an scenario and fail to see it possible

Well, for instance your implementation relies on method-builders and I can't find a reference to that implementation detail anywhere in the reference materials for defprotocol or https://clojure.org/reference/protocols, so it seems like that could be subject to change? I was also worried about the addition of new ways to satisfy but I guess since you call through into the underlying method that's probably safe.

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

No branches or pull requests

2 participants