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

Recommended changes to getting started guide #228

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

Conversation

boazsender
Copy link

It took me a minute to understand that we had moved into the route around the 'compose a zod schema' section of the getting started section. I thought it might help future readers to be more explicit about that.

@netlify
Copy link

netlify bot commented Oct 2, 2023

Deploy Preview for remix-forms ready!

Name Link
🔨 Latest commit 2e0232f
🔍 Latest deploy log https://app.netlify.com/sites/remix-forms/deploys/651b66357185d7000894886b
😎 Deploy Preview https://deploy-preview-228--remix-forms.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.

Comment on lines 250 to 251
it will return structured error messages for us. You can also put this
in the route where you'll be submitting a form.
Copy link
Contributor

@gustavoguichard gustavoguichard Oct 2, 2023

Choose a reason for hiding this comment

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

Actually this is not a good practice as domain functions usually have server side code.
I use to put my schemas in a app/domain/some-domain folder so they can be used in their related domain function. I also keep my domain functions away from the route as there's no need (and often there are problems) to send that code to the client bundle.

Now that we have route folders in Remix v2 you could create those functions in separate files under that route's folder but it is important to make sure you only use them in either loaders or actions so it won't be sent to the client.

Copy link
Author

Choose a reason for hiding this comment

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

Good point. I'm writing short mutations inline in the formAction, but worth clarifying! Let me update the patch.

@gustavoguichard
Copy link
Contributor

gustavoguichard commented Oct 2, 2023

Thank you for taking the time and contributing to this repo!! I appreciate it.

However it is important to keep in mind that any code which is not part of the Remix's magic exports (loader, action, meta, etc) will be sent to the client.

Maybe we should make some changes to clarify that, what do you think @boazsender ?

@boazsender
Copy link
Author

Good call @gustavoguichard. I've tried adding some language. Let me know what you think. Happy to make further changes.

@gustavoguichard
Copy link
Contributor

Looking much better IMO.
@danielweinmann , your call ;)

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.

2 participants