timdeschryver / zod-fixture

Creating fixtures based on zod schemas
https://zod-fixture.timdeschryver.dev/
MIT License
122 stars 10 forks source link

test: generator order #20

Closed timdeschryver closed 1 year ago

timdeschryver commented 1 year ago

I think ideally both tests should succeed.

Maybe look into giving the custom generators priority over the default ones?

THEtheChad commented 1 year ago

I think execution should continue to be the order in which generators are registered, but core has no concept of "defaults". Defaults are provided via the createFixture wrapper. In which case, yes, we need to change this:

export function createFixture<TSchema extends ZodTypeAny>(
    schema: TSchema,
    config: Config & {
        // eslint-disable-next-line @typescript-eslint/no-explicit-any
        extend?: Definition<any>[];
    } = {}
): z.infer<TSchema> {
    return new Core(config)
        .register(defaultGenerators)
        .register(config.extend ?? [])
        .generate(schema);
}

to this

export function createFixture<TSchema extends ZodTypeAny>(
    schema: TSchema,
    config: Config & {
        // eslint-disable-next-line @typescript-eslint/no-explicit-any
        extend?: Definition<any>[];
    } = {}
): z.infer<TSchema> {
    return new Core(config)
        .register(config.extend ?? []) // <------------
        .register(defaultGenerators)
        .generate(schema);
}

Or should we change execution order to match the current configuration? IE run the last registered generators first?

timdeschryver commented 1 year ago

@THEtheChad I think running the first match is better and more intuitive.

timdeschryver commented 1 year ago

(accidently closed 😅)

THEtheChad commented 1 year ago

I made a separate PR based on this one that I think clarifies the intent here. Let me know what you think.

https://github.com/timdeschryver/zod-fixture/pull/23

timdeschryver commented 1 year ago

I think the same intent was added within PR (in https://github.com/timdeschryver/zod-fixture/pull/20/commits/6417fa158b81a73d46876f602a4129d5ec6d874a), or am I missing something? I agree with keeping Core for the flexibility it provides.

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 2.0.0-beta.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: