Fix ObjectSchemaOf type definition

[chasecaleb]

Follow up to #1169, because in my haste to rebase I overlooked adding Lazy to the ArraySchema generic in ObjectSchemaOf. I also simplified the ObjectSchemaOf definition since it doesn't need to repeat the SchemaOf definition. I also fixed my incorrect formatting from #1169.

I've tested this against my own project via yarn link so I know it at least works for my use cases.

[jquense]

I actually don't think you want to add the Lazy here. What is the use case?

[chasecaleb]

Something like this, where the snippet below is used for a field in a Yup.object() validator

Yup.array(
        Yup.lazy<YupSchema<ModuleData>>(element => {
            // ... dynamic logic here
        })
    );

[chasecaleb]

BTW, #1169 already added Lazy to the ArraySchema generic within SchemaOf. This PR just makes it consistent within ObjectSchemaOf as well.

[jquense]

yeah i see. hmm I am a bit concerned that this will break other common cases like #1155

It probably makes sense to have a common interface shared between lazy and schema (there is already one simple one) but unsure if that would break other stuff...

[chasecaleb]

I don't think it breaks that use case, does it? Changing the type definition to add Lazy to the top level like SchemaOf<T> = Lazy<T> | ...the rest would cause issues, but that's not what I'm proposing here. The only part I'm adding Lazy to is inside ArraySchema<SchemaOf<E> | Lazy<SchemaOf<E>>>, not at the top level. So you would still be calling validate() on the ArraySchema type, which the inner generic part shouldn't affect.

[jquense]

yeah that's true for this specific change, but technically it's an incomplete change, since object properties also accept Lazy

so you fix your example above but this still doesn't work i think?

type Person {
   name: string
}

const s: SchemaOf<Person> = object({ name: lazy(() => string().defined() })

[chasecaleb]

Okay, I just committed the ObjectSchemaOf addition. How's it look now?

[jquense]

see the old comment here. We can't use SchemaOf<T[k]> because of how TS applies the types over the conditional in SchemaOf. Basically if the input type is something like string | undefined, you get SchemaOf<string> | SchemaOf<undefined>, not SchemaOf<string | undefined>, The behavior is different for passing the generic to a concrete type like BaseSchema. Just means some duplication

[chasecaleb]

Okay... I'm really confused about what's going on here (and I've been playing with it for two hours now). Either I'm doing something wrong, or it seems like that comment is backwards. Can you look at the minimal example I made in this commit? 8bf7689

[chasecaleb]

I somewhat suspect the issue is with my example code (FormData/buildNestedValidator), not the Yup type definitions... but I can't figure out the right way, either. I'm not sure how to translate my recursive FormData to a corresponding SchemaOf type.

[chasecaleb]

Ah ha, I think I figured out what's going on. I'll get back to you soon with this sorted out. Recursive types are... fun.

[chasecaleb]

Okay, I added a commit that adds lazy to field values for object schemas.

And in case you wanted an explanation on how I got so confused:

interface FormData {
  [K: string]: string | FormData;
}
type FormDataSchema = ObjectSchema<{
  [K: string]: SchemaOf<string> | FormDataSchema;
}>;
export const buildNestedValidator = (
  path: string[],
  validator: SchemaOf<string> | FormDataSchema,
): FormDataSchema => {
  const [head, ...rest] = path;
  const result: SchemaOf<string> | FormDataSchema =
    rest.length === 0 ? validator : buildNestedValidator(rest, validator);
  return objectCreate({ [head]: result });
};
const foo: FormData = { foo: '', bar: { baz: '' } };
const val = buildNestedValidator(['bar', 'baz'], stringCreate().required());
val.validate(foo);

I was trying to use SchemaOf<FormData> originally, but that wasn't working. The solution was to create the FormDataSchema type and use it instead. I think that's because of how the union gets distributed - e.g. SchemaOf<string | string[]> vs SchemaOf<string> | SchemaOf<string[]. Something along those lines - my brain hurts at the moment.

Anyways, that's all irrelevant to the actual change in this PR and is just context on how I got so tripped up testing against my own code base. I went ahead and rebased to squash my mess - this is ready for you to review and potentially merge now.

[chasecaleb]

Hi, any thoughts on the current state of this MR?

[jquense]

will get back to you next week, on vacation!

[chasecaleb]

@jquense quick reminder about this PR. Hope you had a good vacation, too!

[chasecaleb]

Thanks for working with me on this PR and merging it!

Do you have plans to make a new release with this change? It would help me out when you get a chance so that I can get rid of some temporary workaround code, but it isn't urgent.