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

getDefaults sould return [] instead of undefined for arrays #970

Open
notramo opened this issue Dec 10, 2024 · 3 comments
Open

getDefaults sould return [] instead of undefined for arrays #970

notramo opened this issue Dec 10, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@notramo
Copy link

notramo commented Dec 10, 2024

The getDefaults function returns undefined as a default value for array(), but it's not ergonomic. I find myself always doing one of the following:

  • Add unnecessary complication to code that works with the input, to handle undefined, e.g. instead of doing a simple .push(), I have to add an undefined check or a nullish coalescing operator.
  • Add an optional() with default value []

I recommend to set the out-of-the-box default value to [], as it covers the majority of use cases, and if undefined default is needed, there's optional() and undefinedable().

import { getDefaults, object, array, string } from 'valibot';

const mySchema = object({
  myArray: array(string()),
});

console.debug(getDefaults(mySchema));

outputs:

{
  myArray: undefined,
}

better output would be:

{
  myArray: [],
}
@fabian-hiller
Copy link
Owner

Currently, getDefaults only supports object and tuple schemas. Feel free to share your use case so we can investigate whether we should change the current implementation.

Here is a workaround that might work for you in the meantime. Otherwise you can copy the source code of getDefaults and modify it to your needs.

import * as v from 'valibot';

const Schema = v.object({
  array: v.optional(v.array(v.string()), []),
});

const defaults = v.getDefaults(Schema);

@fabian-hiller fabian-hiller self-assigned this Dec 11, 2024
@fabian-hiller fabian-hiller added enhancement New feature or request question Further information is requested labels Dec 11, 2024
@andersk
Copy link
Contributor

andersk commented Dec 22, 2024

This would not be a good change. Perhaps in your particular use case it makes sense to default to [], but in other use cases it will cause problems.

For example, consider an API like

v.union([
  v.object({ allow: v.array(v.string()) }),
  v.object({ deny: v.array(v.string()) }),
]);

where the client specifies either an allowlist {"allow": ["foo", "bar"]} or a denylist {"deny": ["baz", "quux"]}. We cannot accept {}, because there’s no way to tell whether that means to allow nothing or to deny nothing.

As another example, if we’re validating a payload for consumption by some external program we didn’t write, that program might not accept a missing array in lieu of an empty one, so we shouldn’t implicitly accept it either.

The programmer needs to be explicit about which arrays to default to []. v.optional(v.array(…), []) is a fine way to specify that. And for an array that doesn’t default to [], getDefaults should not pretend it does.

@fabian-hiller
Copy link
Owner

Thank you for your feedback! I really appreciate your contributions over the past few weeks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants