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

Optional returns the string | undefined type even with the default value set #286

Closed
NotWorkingCode opened this issue Dec 7, 2023 · 30 comments
Assignees
Labels
bug Something isn't working priority This has priority

Comments

@NotWorkingCode
Copy link

NotWorkingCode commented Dec 7, 2023

I use the following schema:

export const ConfigSchema = v.object({
    logger: v.object({
        level: v.optional(v.string(), "test"),
        path: v.optional(v.nullable(v.string()), null)
    })
})

When I try to pass the value after validation, I get an error like:
TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.   Type 'undefined' is not assignable to type 'string'.

Type: level?: string | undefined

This is a strange behavior, because when the default value is set, either the value from json or the default value will be returned to me.

@fabian-hiller
Copy link
Owner

I have tested your schema in my playground and get the following input and output type:

type ConfigInput = {
    logger: {
        level?: string | undefined;
        path?: string | null | undefined;
    };
}

type ConfigOutput = {
    logger: {
        level: string;
        path: string | null;
    };
}

Which version of Valibot are you using? Can you provide more code so I can investigate the problem? Do you have strict set to true in your tsconfig.json?

@fabian-hiller fabian-hiller self-assigned this Dec 7, 2023
@fabian-hiller fabian-hiller added the question Further information is requested label Dec 7, 2023
@NotWorkingCode
Copy link
Author

Thank you for your support.

struct is indeed set to true

I tried using both Input and Output - the result is the same.

If you use Input
image

If you use Output
image

I am using the latest version: 0.23.0

@NotWorkingCode
Copy link
Author

Now I also noticed that path is always equal to a string, although it can be both a string and null

@fabian-hiller
Copy link
Owner

Is strict set to true in your tsconfig.json?

@NotWorkingCode
Copy link
Author

Is strict set to true in your tsconfig.json?

Yes, I wrote it in my second comment.
For clarity, here is the entire tsconfig
image

@fabian-hiller
Copy link
Owner

I don’t know what the problem is. In my playground everything works as expected.

screenshot-code-1

screenshot-code-2

@fabian-hiller
Copy link
Owner

Just noticed that the question marks are missing in ConfigInput. Will fix that in the next release.

@NotWorkingCode
Copy link
Author

Thank you very much for your work and such a quick response.

@fabian-hiller
Copy link
Owner

v0.24.0 is available. Can you check if the problem still exists on your end?

@NotWorkingCode
Copy link
Author

I get the same results on version 0.24.0

Input
image

Output
image

@fabian-hiller fabian-hiller added bug Something isn't working and removed question Further information is requested labels Dec 11, 2023
@fabian-hiller
Copy link
Owner

You are right. I am sorry. I will fix that.

@fabian-hiller
Copy link
Owner

fabian-hiller commented Dec 11, 2023

v0.24.1 is available. Can you check if it works now? I will soon add unit tests for the types to prevent this from happening again.

@NotWorkingCode
Copy link
Author

Now the behavior has changed, but it still doesn't work correctly.

In my example with nullable, the type is now always a string, and the optional parameter remains a string or undefiend

image

@NotWorkingCode
Copy link
Author

I'll leave my code so you don't have to rewrite it.

https://pastebin.com/4AvtfBBy

@fabian-hiller
Copy link
Owner

Strange. It works as it should for me:

screenshot-github-issue

@fabian-hiller fabian-hiller added the priority This has priority label Dec 11, 2023
@NotWorkingCode
Copy link
Author

Do you think it could be the version of node being used?
I use 18

@fabian-hiller
Copy link
Owner

I don't think so. I also use Node.js v18. It must be related to TypeScript (I use v5.2.2).

@NotWorkingCode
Copy link
Author

I'm using 5.3.2

@fabian-hiller
Copy link
Owner

TypeScript v5.3.2 produces the same results in my editor. Can you clone the Valibot repo and copy and past the following into library/playground.ts and check the output type?

import * as v from './mod.ts';

export const ConfigSchema = v.object({
  logger: v.object({
    level: v.string(),
    format: v.optional(v.string(), '{icon} > {time} | {level} | {message}'),
    path: v.optional(v.nullable(v.string()), null),
  }),
});

export type Config = v.Output<typeof ConfigSchema>;

@NotWorkingCode
Copy link
Author

On playground, I get the correct result, the same as yours
image

@NotWorkingCode
Copy link
Author

I tried to build the project and use the assembled library in playground - the problem returned

image

Apparently, the problem is only observed in the assembled library

@fabian-hiller
Copy link
Owner

You can run cd library && pnpm build to build Valibot. After that you can use the assembled library by import * as v from './dist/index.js'; in playground.ts. There are no bugs for me here either.

@fabian-hiller
Copy link
Owner

fabian-hiller commented Dec 12, 2023

I suspect the bug in the tsconfig.json or the TypeScript or Valibot version used by your editor or package manager.

@NotWorkingCode
Copy link
Author

You can run cd library && pnpm build to build Valibot. After that you can use the assembled library by import * as v from './dist/index.js'; in playground.ts. There are no bugs for me here either.

I did exactly that. After building with pnpm, I imported from dist and the error returned. I showed it in my second screenshot.

@fabian-hiller
Copy link
Owner

This is strange. I don't know what to do. I reinstalled Valibot in another project and tested the code there. Everything works as it should. Since I can't reproduce it, I have no way to debug it. 😐

@NotWorkingCode
Copy link
Author

I created the project on a completely different machine and got the same result

I use tsconfig by default

{
  "compilerOptions": {
    "target": "es2016",
    "module": "commonjs",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "skipLibCheck": true
  }
}

My clear project:
SquadTS.zip

image

@Sec-ant
Copy link

Sec-ant commented Dec 14, 2023

I can partially reproduce this problem in a WebStorm IDE.

image

(You can see the type of path is string not string | undefined in my repro, the correct type should be string | null though)

But I do think this is an IDE bug (or mis-configuration, I don't use WebStorm so I don't know). Moreover, if the format type is what it is reported, then expectString(config.logger.format) should also report errors but it does not, which further suggests the IDE type hinting is somewhat problematic:

image

If this is an IDE bug, this shouldn't affect the build process, and I can confirm no type check errors are emitted when I use npx tsc:

image

So you can still build your project without issues.

And everything works correctly under VSCode.

image

Here is my test code:

import * as v from "valibot";

export const ConfigSchema = v.object({
  logger: v.object({
    level: v.string(),
    format: v.optional(v.string(), "{icon} > {time} | {level} | {message}"),
    path: v.optional(v.nullable(v.string()), null),
  }),
});

declare function expectString(_: string): void;

declare const config: v.Output<typeof ConfigSchema>;

expectString(config.logger.format);

@NotWorkingCode
Copy link
Author

@Sec-ant You're damn right. I switched to VS Code myself a few hours ago and noticed that the problem was solved.
This seems to be a problem with the WebStorm static analyzer.

@NotWorkingCode
Copy link
Author

@fabian-hiller The problem was indeed solved in version 2.24.1

I'm sorry for wasting your time.
Thank you so much for the quick fix.

@fabian-hiller
Copy link
Owner

No problem. Thanks a lot that you found this bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority This has priority
Projects
None yet
Development

No branches or pull requests

3 participants