epicweb-dev / web-forms

Learn the primary mechanism for interactivity on the web.
https://www.epicweb.dev/workshops/professional-web-forms
Other
199 stars 61 forks source link

[WIP] Upgrade conform version #2

Closed edmundhung closed 1 year ago

edmundhung commented 1 year ago

Resolve #1

edmundhung commented 1 year ago

We might need to update the README on exercise 4 as well. Since Conform does type coercion now, there is no need to check if the file is empty.

// Before
const schema = z.object({
    profile: z
        .instanceof(File)
        // When browser constructs a form data from an empty file input, a default file
        // entry would be created. we can validate this by checking the filename and size.
        .refine(file => file.name !== '' && file.size !== 0, 'Profile is required'),
})

// Now
const schema = z.object({
    profile: z.instanceof(File, { message: 'Profile is required' }),
});
kentcdodds commented 1 year ago

We might need to update the README on exercise 4 as well

That's awesome. Yes, let's make that update too 👍 Thanks!

edmundhung commented 1 year ago

This is ready @kentcdodds! The changes are huge so it might be easier to check the commit one by one. Please be aware that the impact for removing the min check is huge, let me know if you want to revert it :)

edmundhung commented 1 year ago

Also, I kept this message in the file as the new report helper isn't helpful for the way you are using it still 😅

kentcdodds commented 1 year ago

@edmundhung, thank you so much for doing this. It's an enormous help at a time where I'm really pressed for time getting all these workshops prepared. I really appreciate it!

Also, I kept this message in the file as the new report helper isn't helpful for the way you are using it still 😅

The diff is so big that it's not auto-focusing that, I don't know what message you're talking about. But I'll review everything post-merge 👍 Thanks a ton!

edmundhung commented 1 year ago

Happy to help anytime! 🤓

The diff is so big that it's not auto-focusing that, I don't know what message you're talking about. But I'll review everything post-merge +1 Thanks a ton!

I meant this message.

kentcdodds commented 1 year ago

Ah, gotcha. Yeah, that's fine :) Thanks!