Array type "empty_values" cannot remove all empty values

Ash Forum Ash Questions / Help
,
1

I have a Phoenix Live component that renders multiple string inputs to match the {:array, :string} Ash type:

image

Previously my codebase was using Ecto, and I’ve found a difference in the way empty_values works between the two: in Ecto, all empty values found in empty_values/0 are removed from the list, whereas in Ash the empty_values constraint is matched directly against the list.

My first guess was to declare my resource attribute as follows:

attribute :skills, {:array, :string}, constraints: [empty_values: [[nil], [""]]]

which works fine if the user does not fill the default blank line on the form.

However, if another blank line is added, or if blank lines are mixed with filled ones, they are inserted as nil in the database:

image
image695×299 5.58 KB

image
image723×136 15.3 KB

image
image1851×173 43.1 KB

I’m looking for a way to trim these values in the same way I would in Ecto, but from reading the documentation and reading the code, it doesn’t seem to me like it’s possible.

I’d like to know if there’s a motivation behind this design choice, or if a discussion & PR to make that possible could be entertained (if so, I’d be more than happy to contribute to this amazing framework :heart:).

Thanks!

2

Honestly, that was added so long ago its hard to say what the original rationale was :laughing:

  1. Ash strings (by default), cast to nil.
  2. We have the nil_items? constraint, which rejects nil values. For us empty_values is a separate kind of thing, representing when the entire list is empty.
  3. I think what you want is a new constraint, called remove_nil_items? that will remove nil items instead of making them an error if they exist.
3

Adding a remove_nil_items? constraint should be pretty straightforward.

But now that you mention it, it’s also unexcepted behavior that nil values make it through to the database if they are initially empty strings.

To circumvent that issue, I guess the is_nil check would benefit from being rerun after applying the item’s type constraints here:

But that would probably be considered a breaking change, right?

Maybe we should continue that discussion in a GitHub issue or draft PR, let me know!

4

I would say that we should move the is_nil check to after applying constraints to the inner items, primarily because types could even theoretically do the reverse, taking a nil item and casting it to a value.

I also wouldn’t personally consider it a breaking change, but rather a bug fix. If nil_items?: false can still produce an array with nil items, that counts as a bug in my book :slight_smile:

Please open an issue for these so we can make sure to address them (or of course a PR :slight_smile: )

1 Like
5

GitHub links for posterity:

1 Like