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

Clarify and maybe change unparse behavior #1123

Open
tomconnors opened this issue Nov 18, 2024 · 4 comments · May be fixed by #1150
Open

Clarify and maybe change unparse behavior #1123

tomconnors opened this issue Nov 18, 2024 · 4 comments · May be fixed by #1150
Labels
enhancement New feature or request help wanted Help most welcome

Comments

@tomconnors
Copy link

As posted on slack, data equal to the result of malli.core/parse cannot be unparsed.

(def int-or-string [:orn [:int :int] [:str :string]])

(malli.core/parse int-or-string 1) ;; -> [:int 1]

(= [:int 1] (malli.core/parse int-or-string 1)) ;; -> true

(malli.core/unparse int-or-string [:int 1]) ;=> :malli.core/invalid

(->> 1
     (malli.core/parse int-or-string)
     (malli.core/unparse int-or-string)) ;; => 1

I have some data in my schema's parsed format, which was not produced by parse, but which I would like to unparse. It appears that's not currently possible.

@ikitommi
Copy link
Member

result of parse is actually a MapEntry. Clojure = matches it to vector of two elements. See https://github.com/metosin/malli/blob/master/src/malli/impl/util.cljc#L8-L9. But, I see this being a problem.

@opqdonut opqdonut added enhancement New feature or request help wanted Help most welcome labels Nov 28, 2024
@opqdonut opqdonut moved this to 👍 Backlog in Metosin Open Source Backlog Nov 28, 2024
@opqdonut
Copy link
Member

To clarify, here's how to get the original example to work:

(malli.core/unparse int-or-string (malli.impl.util/-tagged :int 1)) ; => 1
;; OR
(malli.core/unparse int-or-string (clojure.lang.MapEntry. :int 1)) ; => 1

I agree that this is unwieldy. It would be easy to changed tagged? to accept vectors of size two, but I'm not sure what implications this would have. Are people passing in arbitrary data to unparse? Is there some possibility for confusion?

@opqdonut
Copy link
Member

After thinking about it for a bit, I think this is a safe change. I've prepared a PR (#1140).

opqdonut added a commit that referenced this issue Dec 16, 2024
Using a MapEntry was confusing users, because it printed like a
vector, but you couldn't give a vector to unparse.

The current method of using MapEntry was broken for weird schemas:

```
(def schema
  [:or
   [:tuple :string :keyword]
   [:orn ["any" :keyword]]])

(->> (m/parse schema :any) (m/unparse schema))
; => ["any" :any]
; should've been :any
```

Changes the parse behaviour for (at least) :orn, :altn and :multi

Some place (like the entry parsers) used miu/-tagged to generate
MapEntry values. These use sites now use the new miu/-entry. This
keeps the surface area of this change a lot smaller since we don't
need to touch all the map entry logic.

fixes #1123
replaces #1140
@opqdonut
Copy link
Member

A new solution that uses a custom record for parse output in #1150

opqdonut added a commit that referenced this issue Dec 16, 2024
Using a MapEntry was confusing users, because it printed like a
vector, but you couldn't give a vector to unparse.

The current method of using MapEntry was broken for weird schemas:

```
(def schema
  [:or
   [:tuple :string :keyword]
   [:orn ["any" :keyword]]])

(->> (m/parse schema :any) (m/unparse schema))
; => ["any" :any]
; should've been :any
```

Changes the parse behaviour for (at least) :orn, :altn and :multi

Some place (like the entry parsers) used miu/-tagged to generate
MapEntry values. These use sites now use the new miu/-entry. This
keeps the surface area of this change a lot smaller since we don't
need to touch all the map entry logic.

fixes #1123
replaces #1140
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Help most welcome
Projects
Status: 👍 Backlog
3 participants