-
-
Notifications
You must be signed in to change notification settings - Fork 212
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
Omit removes rest object props #481
Comments
This is the intended behavior. It is documented in our API reference, but I can understand why this might be confusing. I will think about it for further development of the library.
|
@fabian-hiller I've read the note after opening the issue, still didn't figure out the If it's somehow possible to make it work using |
Check out our playground for an example. import * as v from 'valibot';
const ObjectSchema1 = v.object(
{
key1: v.string(),
key2: v.string(),
key3: v.string(),
},
v.unknown()
);
const ObjectSchema2 = v.omit(
ObjectSchema1,
['key2'],
v.unknown() // <-- Add `rest` again
); |
@fabian-hiller thank you for the example. I'm still confused 😅 Here's my playground From what I expect, This key is also removed from autocomplete but we see it's there in the output: I remember trying it with the same result. p.s. it may be the intended behavior and just not the one I'd expect from the code |
Because of the |
@fabian-hiller yeah, it kinda reverts what So basically it's not possible to omit I'd consider this a bug in |
How would you do the same with pure TypeScript? |
@fabian-hiller a quick one: const data = {
a: 'a',
b: 'b',
c: 'c',
d: 'd'
}
function omit<T, K extends keyof T>(object: T, keys: K[]): Omit<T, K> {
const result = { ...object }
keys.forEach((key) => {
delete result[key]
})
return result
}
const result = omit(data, ['a', 'b'])
console.log(result)
console.log(result.c) I'd probably use some kind of |
And how would you implement it if you wanted it to allow unknown entries to pass? I am not sure if I understand you correctly, but I think you want to obmit a specific key, but also allow any unknown key to pass. If that's your goal, there's probably only one solution: You need to explicitly declare that key as |
Yes, thats exactly what I want - omit only specific key and leave the rest untouched (be it specified keys from schema or unknown ones). // initial object
const data = { a: 1, b: 2, c: 3, d: 4 }
// remove ONLY a
// result is { b: 2, c: 3, d: 4 }
omit(data, ['a'])
// remove ONLY a AND c
// result is { b: 2, d: 4 }
omit(data, ['a', 'c']) incorrectI've tried Typescript Omit for correct output type but it's not smart enough for index signature: type Data = {
[key: string]: unknown
a: number
b: number
c: number
d: number
}
type Omitted = Omit<Data, 'a'> The output type loses the overview of defined keys. correctThe correct one can be seen using import type { Except } from 'type-fest'
type Data = {
[key: string]: unknown
a: number
b: number
c: number
d: number
}
type Omitted = Except<Data, 'a'> No wonder type-fest has over 160 million downloads 😅 That's basically what I expect I also believe this is how other libraries work (tested on lodash omit)
|
Same Good read on the topic - microsoft/TypeScript#30825 |
Thank you very much! I will look into this in the next few weeks. Maybe we should also add an |
Hey folks! I have been trying out the latest ( |
This is documented here: https://valibot.dev/api/omit/
I do not recommend your workaround, instead write your schema this way: import * as v from "valibot";
const Schema = v.object({
email: v.pipe(v.string(), v.email()),
password: v.pipe(v.string(), v.minLength(8)),
});
const Without = v.pipe(v.omit(Schema, ["password"]), v.brand("Hello")); |
Thank you for the reply! fwiw, my real use case had a base Schema that included a brand |
Yes, this is the recommended way at this time. I understand that this can be cumbersome in some cases. That's why I will consider adding something like a import * as v from "valibot";
const Schema = v.pipe(
v.object({
email: v.pipe(v.string(), v.email()),
password: v.pipe(v.string(), v.minLength(8)),
}),
v.brand("Hello"),
);
const Without = v.omit(v.removePipe(Schema), ["password"]); What do you think of this idea? What could be alternative names for this function? I prefer short but still meaningful names. |
I do not have a strong preference either way. This was the only case where I thought I needed something like |
Okay. Then let's wait for more feedback from other developers to make a good decision in the long run. Feel free to create a new issue as I plan to close this one. |
@fabian-hiller hey 👋 I've tested latest version and would like you to check if object with rest is working correctly with omit:
|
Yes, |
@fabian-hiller it's very unfortunate it works like this, I'd never expect it to work like this same as other devs from the linked issues above. The docs say:
The result of above example clearly shows that the object still has all selected entries 😱 |
Have a look at the source code in line 407. As written in the docs, |
@fabian-hiller yes, I also expect In the latest example from above I have input data: {
key1: 'a',
key2: 'b',
key3: 'c'
} now omit: omit(objectWithRest, ['key1', 'key3']) here I expect that's why I'd consider it as a bug or a very unreliable method and at least add a note about this behavior in the docs - personally I find the outcome result surprising |
This is actually not the same issue I mentioned when I opened this issue - it works correctly now in latest version. A new issue should be probably opened for tracking |
Rest object props with
unknown()
are removed byomit()
Example:
Current output
Expected output
valibot 0.30.0
The text was updated successfully, but these errors were encountered: