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

Idea: align predicate and base schemas #1092

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

Conversation

frenchy64
Copy link
Collaborator

@frenchy64 frenchy64 commented Aug 18, 2024

It might reduce confusion between e.g., int? and :int if we define one in terms of the other via -proxy-schema.

I realize the point of the the predicates is to look like spec predicates, but it's an easy mistake to make to think :any and any? are synonymous. A better example: for int? it's not obvious it doesn't support :min/max properties.

Another advantage of using proxies here is we can reduce the implementation burden on multimethods that dispatch on m/type. Instead of a defmethod for :any and any?, you could have a defmethod :any and in the default method recur via m/deref to catch the :any case.

It's convenient to proxy things like map? as [:map-of :any :any], I think all current children can stay the same.

This is just a sketch and I have no idea why the test fail.

@NoahTheDuke
Copy link
Contributor

I love this idea and fully support it. I had no idea there were differences and I don't think this is mentioned in the readme.

@ikitommi
Copy link
Member

Agree that there are real problems here:

  1. people use string? instead of :string
  2. lot's of extra dispatching (transformers, generators etc)

Original issue is here and I did a study on this while back, did not push it out as PR it seems 🤔. I'll pull it out and comment more. I thought that solution would remove a lot of clutter (extra mappings in error messages, transformers, generators etc), but was not happy with the end result.

Let's park this PR for now.

@frenchy64 frenchy64 added the hammock need a lot of thinking. might take a while. label Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hammock need a lot of thinking. might take a while.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants