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

discriminated union with dataType json causing SchemaError #519

Closed
konstantinblaesi opened this issue Nov 27, 2024 · 6 comments
Closed

discriminated union with dataType json causing SchemaError #519

konstantinblaesi opened this issue Nov 27, 2024 · 6 comments
Labels
bug Something isn't working next Will be implemented in the next release

Comments

@konstantinblaesi
Copy link

konstantinblaesi commented Nov 27, 2024

Error trace

SchemaError: [addresses.additional.name] No default value specified for field (can be undefined, but must be explicit)
    at Defaults_traverseAndReplace (/projects/sveltekit-superforms/src/lib/errors.ts:290:11)
    at Errors_traverseAndReplace (/projects/sveltekit-superforms/src/lib/errors.ts:237:4)
    at Data_traverse (/projects/sveltekit-superforms/src/lib/errors.ts:221:3)
    at Module.replaceInvalidDefaults (/projects/sveltekit-superforms/src/lib/errors.ts:303:10)
    at Module.superValidate (/projects/sveltekit-superforms/src/lib/superValidate.ts:139:5)
    at /projects/sveltekit-superforms/src/tests/demo.test.ts:55:13
    at runTest (file:///projects/sveltekit-superforms/node_modules/.pnpm/@[email protected]/node_modules/@vitest/runner/dist/index.js:781:11)
    at runSuite (file:///projects/sveltekit-superforms/node_modules/.pnpm/@[email protected]/node_modules/@vitest/runner/dist/index.js:909:15)
    at runSuite (file:///projects/sveltekit-superforms/node_modules/.pnpm/@[email protected]/node_modules/@vitest/runner/dist/index.js:909:15)
    at runFiles (file:///projects/sveltekit-superforms/node_modules/.pnpm/@[email protected]/node_modules/@vitest/runner/dist/index.js:958:5) {
  path: 'addresses.additional.name'
}

From the repro: The testcase bad produces this error trace, testcase good works
Basically the feature is to provide one of three possible address types, something like

addresses.additional.type = { 'type1', 'type2', 'none' }

where the 'none' type has no additional fields

Repro

Code
import { zod } from "$lib/adapters/zod.js";
import { superValidate } from "$lib/superValidate.js";
import { stringify } from "devalue";
import { describe, test } from 'vitest';
import { z } from "zod";

const ZodSchema = z.object({
    addresses: z.object({
        additional: z.discriminatedUnion('type', [
            z.object({
                type: z.literal('poBox'),
                name: z.string()
                    // these constraints cause the test failure
                    // Validation error instead of schema error would be expected?
                    .min(1, "min len").max(10, "max len")
                    .default("")
            }),
            z.object({
                type: z.literal('none'),
            }),
        ])
    })
})
const FormSchema = zod(ZodSchema)
type FormSchema = typeof FormSchema['defaults']

async function validate(data: unknown) {
    const formInput = new FormData()

    formInput.set('__superform_json', stringify(data));
    try {
        await superValidate(formInput, FormSchema)
    } catch (err) {
        console.error(err)
        //
        throw err
    }
}

describe("Demo", () => {
    test("Bad", async () => {
        const data = {
            addresses: {
                additional: {
                    type: 'poBox',
                    name: ''
                }
            }
        } satisfies FormSchema;
        await validate(data)
    })

    test("Good", async () => {
        const data = {
            addresses: {
                additional: {
                    type: 'none'
                }
            }
        } satisfies FormSchema;
        await validate(data)
    })
})

Should this kind of schema work ? Should this be modeled differently?

@konstantinblaesi konstantinblaesi added the bug Something isn't working label Nov 27, 2024
@ciscoheat
Copy link
Owner

You can't stringify __superform_json directly, you need to use devalue. Try that and see if it works. Otherwise you should try to add a default value to the schema, as suggested by the error.

@konstantinblaesi
Copy link
Author

konstantinblaesi commented Nov 29, 2024

I updated the code. The field addresses.additional.name is a required field and I would expect a validation error when empty string is submitted, but instead I get a schema error / crash from superforms ? Where would the default value be placed to fix the issue ? So far I have not had success with default values

@konstantinblaesi
Copy link
Author

This schema

const ZodSchema = z.object({
    addresses: z.object({
        additional: z.discriminatedUnion('type', [
            z.object({
                type: z.literal('poBox'),
                name: z.string()
                    // these constraints cause the test failure
                    // Validation error instead of schema error would be expected?
                    .min(1, "min len").max(10, "max len")
                // .default("")
            }),
            z.object({
                type: z.literal('none'),
            }),
        ])
            .default({ type: 'none', name: '123' })
    })
})

fixes the error and produces this result from superValidate

RESULT {
  id: '1s92os',
  valid: false,
  posted: true,
  errors: {
    addresses: { additional: { name: [ 'min len' ] } }
  },
  data: { addresses: { additional: { type: 'poBox', name: '' } } }
}

but if this is the fix , it is not intuitive since it screems at me in my IDE :D
image

@ciscoheat
Copy link
Owner

There was a problem with the recursive check for discriminated unions, will be fixed in the next release. No default will be required.

@ciscoheat ciscoheat added the next Will be implemented in the next release label Nov 30, 2024
@konstantinblaesi
Copy link
Author

Thanks for the fix! Sadly when assigning a default value on a nested discriminated union things break. Is this expected or should this work?

@ciscoheat ciscoheat reopened this Dec 4, 2024
@ciscoheat
Copy link
Owner

Fixed now with 2.21.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working next Will be implemented in the next release
Projects
None yet
Development

No branches or pull requests

2 participants