Skip to content

Conversation

@Malian
Copy link
Contributor

@Malian Malian commented Oct 31, 2025

This PR ensures preserve_nil_values? is taken into account as a valid option for constraints in Struct type. It also add the same feature in the Map type.

Doc is still not correct

As discussed with @Torkan in the discord channel, the PR keeps the current behavior by setting the option to false by default. That being said, the current doc is not correct (I copied it from the Struct type) because keys with nil value are always stored but the docs says the otherwise.

Current behavior:

%{foo: nil}
# is stored as
{'foo': null}
# is casted to 
%{}

Behavior with preserve_nil_values?: true

%{foo: nil}
# is stored as
{'foo': null}
# is casted to 
%{foo: nil}

Suggestion

Update the doc to says that, for not breaking change policy, the keys will nil value are stored as is, but are not taken into account when casting back to Elixir.

or

Fix the behavior according to the doc, introducing a breaking change in how the value are stored in the database. I'm not sure if this should be considered a breaking change, given that Ash's map or struct values casted from database will not change, only the values stored in the database will change.

How to consider data that have already been stored?

Ultimately, whatever we do, we may be introducing a breaking change. Indeed, people who already use map and struct have data with keys with nil value stored in the database. If those people add the preserve_nil_values?: true option, the data already in the database will now be taken into account when casting to a map or struct potentially overriding another value.

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Documentation changes

@zachdaniel
Copy link
Contributor

Mind running the formatter?

@Malian
Copy link
Contributor Author

Malian commented Nov 2, 2025

@zachdaniel Done! I also fixed a test in the type_test.exs file that was failing because a new key had been added and the error message was no longer valid.

@zachdaniel zachdaniel merged commit eae332d into ash-project:main Nov 4, 2025
38 of 44 checks passed
@zachdaniel
Copy link
Contributor

🚀 Thank you for your contribution! 🚀

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

Successfully merging this pull request may close these issues.

2 participants