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

Primitives values in literal schema #102

Merged
merged 2 commits into from
Aug 21, 2023

Conversation

fvckDesa
Copy link
Contributor

This PR add all primitives values on literal schema.

Solve Issue #97

@netlify
Copy link

netlify bot commented Aug 14, 2023

Deploy Preview for valibot ready!

Name Link
🔨 Latest commit 5e6f084
🔍 Latest deploy log https://app.netlify.com/sites/valibot/deploys/64e35842c6c4c000086c04ef
😎 Deploy Preview https://deploy-preview-102--valibot.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@fabian-hiller
Copy link
Owner

Great! Thanks a lot! I'll check your code as soon as I find time and publish this improvement with the next version.

@fabian-hiller fabian-hiller self-assigned this Aug 14, 2023
@fabian-hiller fabian-hiller added enhancement New feature or request priority This has priority labels Aug 14, 2023
Copy link

@kassiomaia kassiomaia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if null should be a primitive considering that null extends from object!

@fabian-hiller
Copy link
Owner

I think null and undefined should be removed because a literal with this datatype makes no sense. This would also have the consequence that the type Primitive should be renamed to Literal. The tests would then have to be updated. Other than that, the code looks good.

@fvckDesa
Copy link
Contributor Author

I have take the primitives data types from MDN Documentation that includes null and undefined between primitives values.
I think that null and undefined is not very usefull as literal value, but I don't see why someone shouldn't have the possibility to use them

@fabian-hiller
Copy link
Owner

Since this does not change the schema's implementation, it is not so important to me whether we include null and undefined. Both have their advantages and disadvantages. If we leave it like it is, literal is more flexible to use on the one hand. However, the API of Valibot becomes more confusing, on the other hand, as there are now two ways to achieve the same thing. Also, strictly speaking, there is no null and undefined literal since these datatypes cannot be further specified.

Feel free to give me feedback on this again. I'll probably decide tomorrow how I want to proceed.

@fabian-hiller fabian-hiller merged commit 59008c1 into fabian-hiller:main Aug 21, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority This has priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants