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

[Bug]: JsonObject type reports issues in Remix's useSubmit that are technically okay, making workarounds extense #12045

Open
muchisx opened this issue Sep 30, 2024 · 3 comments
Labels

Comments

@muchisx
Copy link

muchisx commented Sep 30, 2024

What version of React Router are you using?

6.26.1

Steps to Reproduce

This is a common usage:

Giving a Prisma schema:

// schema.prisma
model User {
//  ...
	active  boolean @default(true)
}

We want to submit a form that has compatible fields with that schema such as

// userForm.ts
// We want to set the fields as required, even if they can be undefined, because we want to check we are actually getting that field from the form we're creating in the client.
type CreateUserBody {
//  ...
	active: Prsima.UserCreateInput['active'] // boolean | undefined <-- generated by Prisma
}

We then later gather the data, usually a form library or whatever.

// userForm.ts
const data: CreateUserBody = {
//  ...
	active,
}

Finally we do a submit via Remix's useSubmit

// userForm.ts
const submit = useSubmit()

submit(data, {...})
//       👆🏻 // Error, `boolean | undefined is not assignable to JsonValue`

Expected Behavior

undefined should be a valid value on required properties for the JsonValue type below:

type JsonObject = { [Key in string]: JsonValue } & {
[Key in string]?: JsonValue | undefined;
};

Actual Behavior

As described above, required values set as undefined do not pass.

@muchisx muchisx added the bug label Sep 30, 2024
@brophdawg11
Copy link
Contributor

Hm, are you submitting with an encType of application/json or the default of application/x-www-form-urlencoded?

I think if using the latter, the code as it stands today will send a blank string across in FormData for the field.

However, for application/json the tricky part is that the JsonValue is correct because undefined is not a valid JSON value even though it's valid in Javascript objects:

Screenshot 2024-10-02 at 4 19 39 PM

This is important in JSON submissions in RR because we JSON.stringify the submission body into the request we send to your action which will strip any undefined values from the Javascript object:

Screenshot 2024-10-02 at 4 21 29 PM

So something like active: boolean | undefined in your component is actually active?: boolean on the other side of the Request serialization (await res.json()).

So for JSON submissions this feels like the correct typing but for FormData we may want to try to differentiate if it does send the blank string

@muchisx
Copy link
Author

muchisx commented Oct 4, 2024

Hi @brophdawg11 !

Thanks for the reply!

We're using application/json with our submissions, the problem many times is that we have objects that are required and may be undefined such as my example above, and when passing it to the submit function from useSubmit in remix, it warns with TS errors due to the undefined is not assignable to a required key of JsonValue.

The type makes sense when you explain it, as it is defining how the resulting json appears, but the problem is that the submit function has JsonValue as one of the values in the union for the paremeters.

export function useSubmit(): SubmitFunction {

export interface SubmitFunction {
(
/**
* Specifies the `<form>` to be submitted to the server, a specific
* `<button>` or `<input type="submit">` to use to submit the form, or some
* arbitrary data to submit.
*
* Note: When using a `<button>` its `name` and `value` will also be
* included in the form data that is submitted.
*/
target: SubmitTarget,

export type SubmitTarget =
| HTMLFormElement
| HTMLButtonElement
| HTMLInputElement
| FormData
| URLSearchParams
| JsonValue
| null;

Maybe what I'm trying to get at is that JsonValue might not be the correct that parameter that the function SubmitFunction should take in the SubmitTarget ? Maybe another type should be used there ?

Yes the functions serializes the value that is passed onto it but what we pass to the function should be any javascript object of any shape and with any value, regardless if it's required or optional in typescript.

What do you think ?

@brophdawg11
Copy link
Contributor

If we loosen that type on the "input" end, I worry that would open up footguns for this type of (I assume common) usage?

type MyJsonType = {
  value: string | undefined;
};

function Component() {
  let data: MyJsonType = ...
  let submit = useSubmit();
  ...
  // If we allow "looser" json here...
  submit(data, ...);
}

function action({ request }) {
  // ...then we make this incorrect
  let data = (await res.json()) as MyJsonType;
  ...
}

Folks would need to remember to use something like Jsonify in their action which feels easy to forget?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants