redwood: Error when submitting scaffolded form with Int field

When defining a model with an Int field, and then running yarn rw scaffold <name> the generated form doesn’t properly handle that Int field. See screenshot below

Would you be interested in a PR that updates the scaffolding template to look something like this?

const ${singularPascalName}Form = (props) => {
  const onSubmit = (data) => {
<% editableColumns.filter(column => column.type === 'Int').forEach(column => { %>
    data.<%= column.name %> = parseInt(data.<%= column.name %>, 10)
<% }) %>
    props.onSave(data, props?.${singularCamelName}?.id)
  }

Generated code without the change above:

Generated code with the change above (plus some whitespace tweaks):

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 27 (27 by maintainers)

Commits related to this issue

Coerce form values to Prisma type Fixes redwoodjs/redwood#608 — committed to Tobbe/redwood by Tobbe 4 years ago
Coerce form values to Prisma type Fixes redwoodjs/redwood#608 — committed to Tobbe/redwood by Tobbe 4 years ago
Coerce form values to Prisma type Fixes redwoodjs/redwood#608 — committed to Tobbe/redwood by Tobbe 4 years ago
Coerce form values to Prisma type Fixes redwoodjs/redwood#608 — committed to Tobbe/redwood by Tobbe 4 years ago
Coerce form values to Prisma type Fixes redwoodjs/redwood#608 — committed to Tobbe/redwood by Tobbe 4 years ago
Coerce form values to Prisma type Fixes redwoodjs/redwood#608 — committed to Tobbe/redwood by Tobbe 4 years ago
Coerce form values to Prisma type Fixes redwoodjs/redwood#608 — committed to Tobbe/redwood by Tobbe 4 years ago
Coerce form values to Prisma type Fixes redwoodjs/redwood#608 — committed to Tobbe/redwood by Tobbe 4 years ago

Most upvoted comments

@cannikin Thanks for the merge. I’ve created a proper PR from my branch now. Please take a look. https://github.com/redwoodjs/redwood/pull/749

Tobbe on Jun 24, 2020

Now that the PR(s) this one depended on have been merged, I’ll take a look at this again. Most likely today or tomorrow

Tobbe on Jun 21, 2020

Sounds like a great plan. I’m very interested in working on it 🙂 Would it be alright if I focused on 1. and 2. first, and then go for 3. in a separate PR? I haven’t looked at the validation code yet, and would feel more confident doing the other two changes first.

Tobbe on Jun 2, 2020

Old dogs, new tricks, and all that… I was the biggest opponent of TS, and nothing anyone could have told me would have convinced me otherwise. Not until I actually tried it for a couple of projects did I actually start appreciating it. But I think we’ve slipped pretty far away from what this issue was supposed to be about 😄

So, to try to bring it back to point, I could start by creating a PR for our own casting of perhaps only numbers and booleans. Ideally someone would also create a PR for a yup solution. So the two can be compared. But I don’t want to over commit, so I’ll just do one for our own casting first.

Tobbe on May 28, 2020

It seems very anti-Redwood to have the type definitions for your schema defined in four different places:

schema.prisma
GraphQL SDL
Yup schema for form validation
Typescript schema

We’ve got another issue that suggests adding a fifth—validations in the service!

This seems like the opposite of everything we’re trying to do—make your development experience easier. Any ideas @mojombo?

cannikin on May 28, 2020

I’m in bed now, I’ll write up some code tomorrow to show how I think it would/could look with TS support. But yes, another type definition would be needed. Hopefully it can be generated from the Prisma schema though

On Thu, May 28, 2020, 01:08 Rob Cameron notifications@github.com wrote:

Does that mean that the data object that we get in onSubmit would already have the correct types for everything? Thus making this a non-issue?

Maybe? I honestly have next to no knowledge of Typescript and how/where/when types are introduced in this chain. I think the idea of Yup is that it would properly enforce and coerce types, so data would be typed properly, but then where does the type definition for TS come from? Do we then need yet another type definition somewhere? AHHHHH

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/redwoodjs/redwood/issues/608#issuecomment-634990724, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAHQSMJTKFIONSP3UT5OZ3RTWMPTANCNFSM4NLLOMGA .

Tobbe on May 27, 2020

Ahhh that’s right, this was only so that foreign key references to other tables would be handled correctly. Well shoot.

We may want to coerse more than ints in the future…what you do think about a higher level check that avoids having to do it inline in the onSubmit handler? We’ve been looking at integrating Yup which would allow you to define the datatypes of all the fields in your form and then the data object returned to onSubmit would have the proper datatypes automatically: https://react-hook-form.com/advanced-usage/#SchemaValidation

I’m not thrilled about yet another place you have to define your schema datatypes (schema.prisma and GraphQL SDL are already two places), but this Int issue has come up several times and we’ve got another issue about dealing with Booleans. Adding Yup wouldn’t add any overhead on scaffolds since we can generate that schema for you. We’ll just have to let people know to either write a coercement manually in onSubmit, as we’ve been doing above, or write themselves a Yup schema.

Maybe we can even add a little CLI tool that turns the model definition in your schema.prisma into the equivalent Yup schema and outputs it to the terminal. Then you can copy/paste into your component…

@thedavidprice your Yup dreams may be coming true!

cannikin on May 27, 2020