Skip to content

Add null value and Setting type #2478

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

Merged
merged 1 commit into from
Apr 3, 2024
Merged

Add null value and Setting type #2478

merged 1 commit into from
Apr 3, 2024

Conversation

swallez
Copy link
Member

@swallez swallez commented Apr 3, 2024

This PR adds a NullValue type that is an alias to null, meant to represent "meaningful" null values, i.e. nulls that are not equivalent to missing value.

It also introduces a Settings type that represent a setting. Elasticsearch handles settings in very specific ways:

  • setting their value to null resets the setting to its default value
  • because of how they're implementing on the server, setting values are always returned as strings.

This PR does not uses these new types and value in the API specification. They first need to be implemented in code generators where relevant (a separate issue will track this).

Fixes #2049 - from the proposals in this issue, we introduce a specific type for meaningful nulls, but keep the standard typescript null for "null as optional".

@swallez swallez requested a review from a team as a code owner April 3, 2024 13:11
Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Maybe we should as well an a behavior that marks a complete class as "Settings". The IndexSettings is a pretty huge type for example and we would need to add the new marker interface to each property.

@swallez
Copy link
Member Author

swallez commented Apr 3, 2024

@flobernd good point the Settings behavior. I hesitated to add it to this PR, but finally decided to keep it small so that we can unlock progress on #2367.

We'll add it in a separate PR, and it will indeed avoid heavy modifications of the various setting types. However, I consider this Settings behavior as something that should be processed during the compilation phase by updating every property of a scalar type to be a Setting<T> so that we don't have to implement that in every code generator. So a bit of additional work that also mandates a separate PR. WDYT of this approach?

@swallez swallez merged commit 141c83e into main Apr 3, 2024
@swallez swallez deleted the null-as-value branch April 3, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Represent "meaningful null"
2 participants