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(Form): fix valibot imports #617

Merged
merged 1 commit into from
Sep 8, 2023
Merged

fix(Form): fix valibot imports #617

merged 1 commit into from
Sep 8, 2023

Conversation

romhml
Copy link
Collaborator

@romhml romhml commented Sep 7, 2023

Fixes import errors from valibot in the Form component by importing safeParseAsync dynamically.

ERROR  Failed to resolve import "valibot" from "../../node_modules/.pnpm/@nuxt+ui@2.8.0_ts-node@10.9.1_vue@3.3.4_webpack@5.88.2/node_modules/@nuxt/ui/dist/runtime/components/forms/Form.vue". Does the file exist?

@vercel
Copy link

vercel bot commented Sep 7, 2023

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

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 7, 2023 2:33pm

@benjamincanac benjamincanac merged commit 1a7eb27 into nuxt:dev Sep 8, 2023
3 checks passed
@benjamincanac
Copy link
Member

Thanks for the fix! Got a bit overwhelmed with the release and the rebranding yesterday, I forgot to try this myself 😅

@ZainW
Copy link

ZainW commented Sep 8, 2023

im still having a similar issue on the edge release which includes this commit

CleanShot 2023-09-08 at 08 42 59@2x

@ZainW
Copy link

ZainW commented Sep 8, 2023

because of this issue there are no working versions of nuxt/ui right now, old package (nuxtlabs) cannot be found and the only version published under the nuxt/ui package name is broken, same with edge.

will install valibot in the meantime as a workaround

Copy link
Member

@ZainW Would you mind sharing a reproduction on StackBlitz of your code and the edge version?

@romhml
Copy link
Collaborator Author

romhml commented Sep 8, 2023

Yeah I'm also experiencing the issue on the latest edge version... I thought that dynamic imports would do the trick but I must have missed something.

I'll look into it.

Copy link
Member

@ZainW Also, what do you mean old package cannot be found? We deprecated @nuxthq/ui but its still available on NPM: https://www.npmjs.com/package/@nuxthq/ui

@madebyfabian
Copy link
Sponsor

This is... not good 😄 experiencing the same. Here is a reproduction with the latest edge installed:
https://stackblitz.com/edit/nuxt-ui-rz5q3s?file=app.vue,package.json

@onemid
Copy link

onemid commented Sep 9, 2023

Maybe you can install the valibot manually to workaround.
I solved this problem by npm install valibot

@jduartea
Copy link
Contributor

jduartea commented Sep 9, 2023

Hey @romhml, I managed to make this work without importing safeParseAsync. Basically using schema._parse(state) instead of safeParseAsync(schema, state)

@romhml
Copy link
Collaborator Author

romhml commented Sep 9, 2023

@jduartea that could do the trick, looks like the only thing safeParseAsync does is call _parse and throw if there's any issue (https://github.com/fabian-hiller/valibot/blob/98ef1ccd4dd3726aa85076c2f0bcb912765ad9c0/library/src/methods/parse/parseAsync.ts#L18), but since the function is private there's no guarantee that it'll stay that way.

My solution would be to rollback and provide an example to implement it into your own component. #639

@benjamincanac what do you think?

@romhml
Copy link
Collaborator Author

romhml commented Sep 9, 2023

Here is an example with a custom component: https://stackblitz.com/edit/nuxt-ui-3ct9hb?file=components%2FBaseForm.vue

Copy link
Member

@romhml Are you sure merging #640 is not enough? We can still find another solution if valibot gets an update.

@romhml
Copy link
Collaborator Author

romhml commented Sep 9, 2023

@benjamincanac It work. That's your call, I'd avoid relying on private functions if you ask me, but it should be manageable in this case.

Copy link
Member

I agree with you, but in this case all the parseAsync method is doing is call the schema._parse.

Also, I don't fell confortable reverting this feature as people started using it already. It's on me for merging the PR too fast and introducing this in 2.8.0 release.

@Zisan-uzum
Copy link

Maybe you can install the valibot manually to workaround. I solved this problem by npm install valibot

I really appreciate it solved

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.

None yet

7 participants