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

Add transformValues option to performMutation and formAction #110

Merged
merged 5 commits into from
Nov 29, 2022

Conversation

danielweinmann
Copy link
Contributor

@danielweinmann danielweinmann commented Nov 29, 2022

Check out the new "Transform values" example for understanding what the functionality does.

The primary use case is to pass URL input to mutations without having to add a hidden field to the form. Let's say we're at /users/$userId/edit. Before transformValues, we'd have to add the userId field to the form schema, get the value from the params, pass it to the Form's values prop, and mark the userId field as hidden.

Now we can leave the userId out of the form schema altogether, add it only to the mutation schema, and pass transformValues: (values) => ({ ...params, ...values }) to our formAction or performMutation.

@netlify
Copy link

netlify bot commented Nov 29, 2022

Deploy Preview for remix-forms ready!

Name Link
🔨 Latest commit fd42209
🔍 Latest deploy log https://app.netlify.com/sites/remix-forms/deploys/63864769ba7298000812de75
😎 Deploy Preview https://deploy-preview-110--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 settings.

Copy link
Contributor

@gustavoguichard gustavoguichard left a comment

Choose a reason for hiding this comment

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

I like it! Then I can use a pattern I'm used to - which I call: "let zod handle the params"

export async function action({ request, params }: ActionArgs) {
  return formAction({
    request,
    mutation,
    schema,
    additionalInput: params,
  })
}

That said I'm not sure about the naming... it feels to me we could come up with some better name but I'm out of ideas right now 😅


const mutation = makeDomainFunction(mutationSchema)(async (values) => values)

export const action: ActionFunction = async ({ request }) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we use ActionArgs here for better type inference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only if we want to promote Remix + TS best practices on the docs, which I think might be valid. But we don't need it with Remix Forms, because Remix Forms is the one that calls useActionData and makes sure the typing is correct. We'll almost never call useActionData<typeof action> when using Remix Forms.

For now, I've been preferring the shorter code that arrow functions give us. I already feel there is a lot of code to read in the examples, so I take every opportunity I have to have fewer lines of code :)

But I'm very open to changing this if you really care about promoting better Remix TS practices on the docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, ship it

@danielweinmann danielweinmann changed the title Add additionalInput option to performMutation and formAction Add transformValues option to performMutation and formAction Nov 29, 2022
@danielweinmann
Copy link
Contributor Author

I like it! Then I can use a pattern I'm used to - which I call: "let zod handle the params"

export async function action({ request, params }: ActionArgs) {
  return formAction({
    request,
    mutation,
    schema,
    additionalInput: params,
  })
}

That said I'm not sure about the naming... it feels to me we could come up with some better name but I'm out of ideas right now 😅

@diogob suggested a broader API with a transformValues function instead. I liked his suggestion and implemented it. What do you think, @gustavoguichard?

@gustavoguichard
Copy link
Contributor

Cool! It reminds me of Zod preprocess, another possible name but I like the idea, opens up for more possibilities

diogob
diogob previously approved these changes Nov 29, 2022
Copy link
Contributor

@diogob diogob left a comment

Choose a reason for hiding this comment

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

LGTM

@danielweinmann danielweinmann merged commit a34272f into main Nov 29, 2022
@danielweinmann danielweinmann deleted the additional-input branch November 29, 2022 18:40
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