Fork me on GitHub
#malli
<
2020-01-12
>
ikitommi11:01:42

Nice, date-support is on malli backlog, need to figure out a way to make it work with cljs too.

roklenarcic15:01:53

Generally, transform operations do not validate that input passes the validator first, which is good for performance, but when you have an operation like :or how do you know which branch of OR to use for transformation? Currently you try every branch as transformation and return the first result that changes the value.

roklenarcic15:01:58

however this leads to this:

roklenarcic15:01:03

(m/encode [:or
           [string? {:encode/string #(.toUpperCase %)}]
           [int? {:encode/string str}]]
          1
          mt/string-transformer)

roklenarcic15:01:15

Execution error (IllegalArgumentException) at com.github.roklenarcic.malli-inline/eval17000$fn (form-init15839185010501867114.clj:2).

roklenarcic15:01:29

No matching field found: toUpperCase for class java.lang.Long

roklenarcic15:01:16

none of the built-in transformers will validate input and return the value itself if not the right type, which would make :or work correctly in this regard

roklenarcic15:01:12

The only realistic way to fix this would be to have :or do validate for each branch until it finds one that validates and then run that transform. This would perform as expected, I can prepare a PR.

ikitommi18:01:57

@roklenarcic sounds right to me to select branch based on validate

roklenarcic18:01:21

another possible bug

roklenarcic18:01:57

in about 5 different explainer functions in malli.core you have distance (if (seq properties) 2 1)

roklenarcic18:01:16

if properties is missing then distance should be 1

roklenarcic18:01:40

the problem is that if someone puts {} as properties, then (seq properties) is false

roklenarcic18:01:03

and it will say distance 1, when the form has [symbol {} children]

roklenarcic18:01:30

so the path is possibly wrong… I don’t know what the semantics of path is exactly

ikitommi18:01:15

get-in to m/form of the schema with the error :in should point to schema

ikitommi18:01:46

needed in pretty error reporting , which is wip.

roklenarcic18:01:48

but you see what I’m getting at… (seq properties) is nil for nil and {}, not good for counting elements in form

ikitommi18:01:22

yes, that's currently wrong but should be easy to fix

ikitommi18:01:34

Good thing that it's pre-alpha ;)

ikitommi18:01:20

Will need malli at a project starting tomorrow, so good reason to push out first alpha too

ikitommi20:01:23

should the schemas allow nil children? e.g. [:and int? nil pos-int?]. Currently :map allows, composite schemas don’t.

ikitommi20:01:11

e.g. [:map [:x int?] nil [:y pos-int?]] works, [:and int? nil pos-int?] doesn’t.

ikitommi20:01:26

all core schemas should behave identically.

ikitommi20:01:33

also, currently:

(m/form [:and {} int?])
; => [:and int?]

(m/properties [:and {} int?])
; => {}

ikitommi20:01:22

should empty properties be removed (set to nil) already when schema is created from AST?

ikitommi20:01:44

after that:

(m/form [:and {} int?])
; => [:and int?]
(m/properties [:and {} int?])
; => nil

ikitommi20:01:28

my suggestion: • allow nil child schemas in all core schemas: easy to create programmatically (e.g. [:map [:x int?] (if require-y [:y int?])]) • treat empty properties as nil, allows (`[:map (if close-maps {:closed true}) [:x int?]]`)

roklenarcic20:01:23

sure, sounds sensible

roklenarcic22:01:35

can explainer fn return null? I thought it must return acc

pithyless23:01:10

I agree all core schemas should behave identically; I would suggest not ignoring nil children, which gives more obvious semantics in cases like this: [:and int? nil pos-int?] (this would always be false, instead of implementing a special-case for nil that behaves differently than regular "falsey" semantics in Clojure). In the map example, I find it more intuitive to use malli.util/merge semantics to describe programmmatic optionality: (malli.util/merge [:map [:x int?]] (if require-y [:map [:y int?]]))