-
Notifications
You must be signed in to change notification settings - Fork 20
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
allowUnkownKeys()
causes all optional()
keys to be set to undefined
?
#42
Comments
Actually, upon diving back into the code to investigate, I have found the reasoning as to why this is the way it is. Suppose this: const schema = mz.object({ name: mz.string(), age: mz.number() })
schema.parse({ name: 'Bob', age: 42, password: 'secret' }) // throws Here we expect it to throw because it not of the type we wanted. Technically it is a subtype, but one of the original aims of myzod was to be strict and enforcing. That way there is no situation (although extremely rare) where you are manipulating data that you did not intend to once parsed by a myzod schema. The allowUnknown option is intended to not throw in the above case but is not meant to passthrough the unknown fields because myzod wants to return to you exactly the type you defined. If you want to allow unknown keys and let them be passthrough in the schema you would do that via a keySignature: const schema = mz.object({ name: mz.string(), age: mz.number(), [mz.keySignature]: mz.unknown() })
schema.parse({ name: 'Bob', age: 42, password: 'secret' }) // works and keeps field password The difference is now that the type system lets you know you may have unkown fields and can help you when reducing or mapping over keys or the likes. So it is indeed more type-safe than zod, and intentional. In the first case: const myschema = myzod.object({ foo: myzod.string().optional() });
myschema.parse({}); // returns {} myzod is simply smart enough to know the schema does not need to modify its inputs and so returns the same object but typed. Does this make sense? |
And the mechanism is to coerce the object to the correct shape when allowUnknown is true and we need to strip keys, and when it coerces it adds the missing key to the object. A small side-effect, but still consistent from a type system perspective. |
I tried this but it doesn't provide correct typing: const schema = mz.object({ foo: mz.string(), [mz.keySignature]: mz.unknown() });
export type Schema = Infer<schema>; results in Schema getting typed as: type Schema = {
[x: string]: unknown;
} So all the keys are being clobbered. Maybe that's an entirely different bug?
But that results in completely unexpected behavior, I think. I would never expect that just by using Is there a workaround at least? This is a huge problem for me because in my use case (firebase, in this particular instance) there is a distinct difference between a value having an explicit |
I have the correct type being inferred from the keysignature based schema: { foo: sting; [x: string]: unknown } What is your tsconfig and typescript versions?
You are correct that this behaviour is odd. I will try and commit a fix for this. The work around is the keySignature. it is weird that it is not working for you. |
I tried various configs. Using the This is Typescript 4.2.4 Upon further investigation, it seems to depend on my import. Following the examples in the import myzod, { Infer } from 'myzod'; I get the clobbered, unexpected type signature. But if I import this way (as is being done in the specs): import * as myzod from 'myzod'; I get the correct typing that you are seeing. |
that's very odd because if I add this test it passed: import myzod from '../src'
import * as z from '../src'
it('default keysignature should be the same as named keysignature', () => {
assert.deepStrictEqual(myzod.keySignature, z.keySignature);
}); but you seem to be correct in the behaviour... |
I just published v1.7.4 which I believe should fix the type error, I will test more carefully later when I have more time. but just in case it works properly mentioning it here |
It is very strange. I guess you'd have to actually add a check for the types produced to verify this sort of thing. Not sure if it's related, but I see an error in VS code when using
But checking through |
it's actually not strange in the end because that test is testing the equality of those values in JS world. It was the type that was wrong. The default export: export default {
keySignature // Symbol
} Lost the type information associated to the uniqueness of the symbol and just had Typescript is the gift that keeps on giving. I consider this solution to be not the workaround but the proper way to accept unknown keys in myzod; by typing them as unknown. Please let me know if this fixes your short term problem. I will investigate not setting optional keys for allowUnknown schemas because you are right it is an unexpected side effect, however it will incur a performance hit and depending on the result I may choose to fix or not fix it. Thanks for reporting this and being patient! |
This workaround works great, thanks. Thanks for being so responsive! |
I just wanted to update this with some additional, similar behavior I'm seeing, which I'm pretty sure is related. First, using const myzod = require("myzod");
const data = { id: 1, a: "apple", b: "banana" };
myzod.object({ id: myzod.number(), c: myzod.string().optional() }).allowUnknownKeys().parse(data);
// { id: 1, c: undefined }
myzod.object({ id: myzod.number().coerce(), c: myzod.string().optional(), [myzod.keySignature]: myzod.unknown() }).parse(data);
// { id: 1, a: 'apple', b: 'banana', c: undefined }
// It doesn't matter what map does (even if it does nothing) for this occur
myzod.object({ id: myzod.number().map(a => a), c: myzod.string().optional(), [myzod.keySignature]: myzod.unknown() }).parse(data);
// { id: 1, a: 'apple', b: 'banana', c: undefined } I think you may have eluded to this behavior in a previous comment in regards to But further -- and this is where it's a little "weird" -- any "parent schemas" are also affected by this This requires a bit more complex data structure to illustrate, so I threw together a set of random test data using Mockaroo: [
{
"id": 1,
"lastName": "Bynert",
"gender": "Polygender",
"ipAddress": "15.57.73.220"
},
{
"id": 3,
"otherId": 92,
"firstName": "Marcelle",
"lastName": "Ebbett",
"email": "mebbett2@uiuc.edu",
"gender": "Female",
"ipAddress": "5.73.241.192",
"job": {
"title": "Desktop Support Technician",
"department": "Business Development",
"company": {
"id": "1024921263",
"name": "Livetube"
}
}
},
{
"id": 6,
"firstName": "Percy",
"lastName": "Hadye",
"gender": "Polygender",
"ipAddress": "187.142.139.82",
"job": {
"title": "Account Executive",
"department": "Marketing",
"company": {
"id": "8633553945",
"name": "Lajo"
}
}
},
{
"id": 7,
"firstName": "Damian",
"lastName": "Herrema",
"email": "dherrema6@paypal.com",
"gender": "Female",
"ipAddress": "101.113.242.176",
"job": {
"title": "Pharmacist",
"company": {
"id": "3772053157",
"name": "Fadeo"
}
}
},
{
"id": 8,
"lastName": "Trunchion",
"gender": "Male"
},
{
"id": 10,
"otherId": 68,
"firstName": "Masha",
"lastName": "Amori",
"gender": "Agender",
"ipAddress": "40.238.59.24",
"job": {
"title": "VP Marketing",
"department": "Training",
"company": {
"id": "736441389",
"name": "Fanoodle",
"employees": 30
}
}
}
] And started with the following code: import myzod from "myzod";
import data from "./data.json";
const companySchema = myzod.object({
id: myzod.string().optional(),
name: myzod.string().optional(),
employees: myzod.number().optional(),
});
const jobSchema = myzod.object({
title: myzod.string().optional(),
department: myzod.string().optional(),
company: companySchema.optional(),
});
const personSchema = myzod.object({
id: myzod.number(),
otherId: myzod.number().optional(),
firstName: myzod.string().optional(),
lastName: myzod.string(),
email: myzod.string().optional(),
gender: myzod.string(),
ipAddress: myzod.string().optional(),
job: jobSchema.optional(),
});
const dataSchema = myzod.array(personSchema);
const result = dataSchema.parse(data);
console.dir(result, { depth: null, colors: true }); As it stands, this all works "as expected" -- no {
id: 7,
firstName: 'Damian',
lastName: 'Herrema',
email: 'dherrema6@paypal.com',
gender: 'Female',
ipAddress: '101.113.242.176',
job: {
title: 'Pharmacist',
company: { id: '3772053157', name: 'Fadeo' }
}
} But if {
id: 7,
otherId: undefined,
firstName: 'Damian',
lastName: 'Herrema',
email: 'dherrema6@paypal.com',
gender: 'Female',
ipAddress: '101.113.242.176',
job: {
title: 'Pharmacist',
department: undefined,
company: { id: '3772053157', name: 'Fadeo' }
}
} Note that Moving down the tree: if you add a {
id: 7,
otherId: undefined,
firstName: 'Damian',
lastName: 'Herrema',
email: 'dherrema6@paypal.com',
gender: 'Female',
ipAddress: '101.113.242.176',
job: {
title: 'Pharmacist',
department: undefined,
company: { id: '3772053157', name: 'Fadeo', employees: undefined }
}
} To further illustrate that this only affects the schema and it's parent schemas, but not child schemas: f {
id: 7,
otherId: undefined,
firstName: 'Damian',
lastName: 'Herrema',
email: 'dherrema6@paypal.com',
gender: 'Female',
ipAddress: '101.113.242.176',
job: {
title: 'Pharmacist',
company: { id: '3772053157', name: 'Fadeo' }
}
} So the additional unexpected behavior is that this affects "parent schemas". And also that |
So, yeah. Certain types or schemas are coercable. A mapped type is by definition coercing a type to another type. Or at least performing some transformation on the data within the same type. When a parent type includes a type that is coerceable than the parent also is coerceable. Suppose: const schema = mz.object({ age: mz.number().coerce() })
const result = schema.parse({ age: "23" }) // { age: 23 } A coercion from types { age: string } to { age: number } happened between the input and the result of parsing. So when a parent has a child schema that is coerceable by any means it sets itself as coerceable. How it does not set all of it's child schemas as coerce-able if it does not have to. It does lead to this inconsistency of sorts about when exactly the undefined key will be added to the result. So in the end none of it was necessarily designed on purpose to have this inconsistency but as a consequence of other priorities. Mentally as you have figured out you can know when myzod will add the key to the result by determining if the type is a "coerceable". My position is that regardless of if the key is not present or is present but set to undefined, it still respects the type-system. I am keeping up this issue for documentation purposes, but you are convincing me that I may need to add a warning about this to the readme and a small section on using keySignatures if you need to preserve the absence of certain keys. |
Given:
The result is an empty object (
{ }
)But if I add
allowUnknownKeys()
:This results in
{ foo: undefined }
This isn't expected behaviour, is it?
Further, shouldn't
allowUnknownKeys()
not strip out the unknown keys, and pass them through?Maybe coming from
zod
I'm expecting the behavior to be similar?The text was updated successfully, but these errors were encountered: