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

fix(docs): update valibot snippet #6569

Merged
merged 5 commits into from
Jun 18, 2024
Merged

Conversation

zougari47
Copy link
Contributor

Overview

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Description

I was trying to implement modular forms like on docs but when I installed valibot ^0.32.0 found there is no exported type Input (I think it's outdated).
When I looked in valibot docs they use InferOutput instead.

Use cases and why

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

@zougari47 zougari47 requested a review from a team as a code owner June 18, 2024 13:49
Copy link

netlify bot commented Jun 18, 2024

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 467b956

@zougari47
Copy link
Contributor Author

const LoginSchema = object({
  email: string([
    minLength(1, 'Please enter your email.'),
    email('The email address is badly formatted.'),
  ]),
  password: string([
    minLength(1, 'Please enter your password.'),
    minLength(8, 'Your password must have 8 characters or more.'),
  ]),
});

I'm not a Valibot expert but this snippet from docs gives this error:

Argument of type '(MinLengthAction<LengthInput, 1, "Please enter your email."> | EmailAction<string, "The email address is badly formatted.">)[]' is not assignable to parameter of type 'ErrorMessage<StringIssue> | undefined'.ts(2345)

When I wrote it that way 👇. The error removed but I'm not sure if it's the same thing.

const LoginSchema = object({
  email: pipe(
    string(),
    minLength(1, 'Please enter your email'),
    email('he email address is badly formatted'),
  ),
  password: pipe(
    string(),
    minLength(1, 'Please enter your password.'),
    minLength(8, 'Your password must have 8 characters or more.'),
  ),
})

@gioboa
Copy link
Member

gioboa commented Jun 18, 2024

const LoginSchema = object({
  email: string([
    minLength(1, 'Please enter your email.'),
    email('The email address is badly formatted.'),
  ]),
  password: string([
    minLength(1, 'Please enter your password.'),
    minLength(8, 'Your password must have 8 characters or more.'),
  ]),
});

I'm not a Valibot expert but this snippet from docs gives this error:

Argument of type '(MinLengthAction<LengthInput, 1, "Please enter your email."> | EmailAction<string, "The email address is badly formatted.">)[]' is not assignable to parameter of type 'ErrorMessage<StringIssue> | undefined'.ts(2345)

When I wrote it that way 👇. The error removed but I'm not sure if it's the same thing.

const LoginSchema = object({
  email: pipe(
    string(),
    minLength(1, 'Please enter your email'),
    email('he email address is badly formatted'),
  ),
  password: pipe(
    string(),
    minLength(1, 'Please enter your password.'),
    minLength(8, 'Your password must have 8 characters or more.'),
  ),
})

@fabian-hiller you are the master here 😄 can you have a look pls?

@fabian-hiller
Copy link
Contributor

Thanks @zougari47! Can you also change the imports to import * as v from 'valibot';? Here is more context:

@zougari47
Copy link
Contributor Author

Thanks @zougari47! Can you also change the imports to import * as v from 'valibot';? Here is more context:

* https://valibot.dev/blog/valibot-v0.31.0-is-finally-available/

* https://valibot.dev/guides/migrate-to-v0.31.0/

Done ✅. Thank you for the recomandation.

@fabian-hiller
Copy link
Contributor

Can you also update this file? https://github.com/QwikDev/qwik/blob/main/packages/docs/src/routes/demo/integration/modular-forms/index.tsx

Copy link

pkg-pr-new bot commented Jun 18, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

commit: 467b956

@builder.io/qwik

npm i https://pkg.pr.new/@builder.io/qwik@6569

@builder.io/qwik-city

npm i https://pkg.pr.new/@builder.io/qwik-city@6569

eslint-plugin-qwik

npm i https://pkg.pr.new/eslint-plugin-qwik@6569

create-qwik

npm i https://pkg.pr.new/create-qwik@6569

gioboa
gioboa previously approved these changes Jun 18, 2024
Copy link
Member

@gioboa gioboa left a comment

Choose a reason for hiding this comment

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

Thanks @zougari47 for your help and @fabian-hiller for the review 👏

gioboa
gioboa previously approved these changes Jun 18, 2024
@fabian-hiller
Copy link
Contributor

One last thing. Can you change v.minLength(1, 'Please enter your email.') to v.nonEmpty('Please enter your email.')?

@zougari47
Copy link
Contributor Author

One last thing. Can you change v.minLength(1, 'Please enter your email.') to v.nonEmpty('Please enter your email.')?

Done ✅. Thank you for the review.

@gioboa gioboa enabled auto-merge (squash) June 18, 2024 23:16
@gioboa gioboa merged commit e10f74d into QwikDev:main Jun 18, 2024
19 of 20 checks passed
@zougari47 zougari47 deleted the update-docs-valibot branch June 21, 2024 12:47
genki pushed a commit to genki/qwik that referenced this pull request Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants