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

docs: improve nullable/nullish type definitions #1008

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

frenzzy
Copy link

@frenzzy frenzzy commented Jan 8, 2025

Updates the JSDoc documentation for nullable() and nullish() functions to better illustrate their type behavior.

Changes:

  • Added TypeScript union types (T | null vs T | null | undefined) to clearly show the difference

This change makes it easier for developers to quickly understand (especially non english speaking) when to use each function based on their TypeScript type requirements.

P.S.: Is it only me who keeps googling "nullable vs nullish difference" every other week? 😅

Copy link

vercel bot commented Jan 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
valibot ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 9, 2025 6:53am

Copy link

pkg-pr-new bot commented Jan 8, 2025

Open in Stackblitz

npm i https://pkg.pr.new/valibot@1008

commit: cfc2c2b

@fabian-hiller
Copy link
Owner

I don't think I want to merge this PR. I agree that your changes make it clearer, but the current approach is consistent across all of our actions. In simple words: We use almost exactly the same description template for every JSDoc, and I think I prefer consistency in this case. If we decide to change the JSDocs it should be consistent across all actions and probably all other functions too.

@frenzzy
Copy link
Author

frenzzy commented Jan 9, 2025

Ok, I agree it's good to have consistent docs, i.e., it would be better to add examples to all the functions.

But what about the latest code suggestion with just minor wording improvements?

  • Creates a nullish schema that allows null and undefined values.
  • Creates a nullable schema that allows null values.

Alternatively, for consistency, we could just add parentheses (they don't have to be everywhere):

  • Creates a nullish schema (T | null | undefined).
  • Creates a nullable schema (T | null).

@fabian-hiller
Copy link
Owner

Your ideas are good. I will come back to this and other similar issues and PRs after V1 is available.

@fabian-hiller fabian-hiller self-assigned this Jan 13, 2025
@fabian-hiller fabian-hiller added the documentation Improvements or additions to documentation label Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants