react-router
react-router copied to clipboard
[Bug]: `JsonObject` type reports issues in Remix's `useSubmit` that are technically okay, making workarounds extense
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:
https://github.com/remix-run/react-router/blob/fab7723cff7ca970d84b30c422971a35a93c514c/packages/react-router-dom/dom.ts#L118-L120
Actual Behavior
As described above, required values set as undefined do not pass.
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:
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:
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
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.
https://github.com/remix-run/react-router/blob/fab7723cff7ca970d84b30c422971a35a93c514c/packages/react-router-dom/index.tsx#L1566
https://github.com/remix-run/react-router/blob/fab7723cff7ca970d84b30c422971a35a93c514c/packages/react-router-dom/index.tsx#L1519-L1529
https://github.com/remix-run/react-router/blob/f511c220cede81f52129e9fb2c24af922c0c3dd2/packages/react-router-dom/dom.ts#L125-L132
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 ?
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?
To align with our new Open Governance model, we are now requiring that all issues have a minimal and runnable reproduction. To that end, we're doing some housekeeping in the repo to clean up existing issues that do not have a valid reproduction. This should get us down to a more manageable number of issues and allow us to be more responsive to existing and newly filed issues.
We're using a GitHub actions script to identify issues without a reproduction by looking for a StackBlitz, CodeSandbox, or GitHub link in the issue body. This won't be perfect, so if this issue has a reproduction on another platform, please comment back on here, and we can re-open the issue. Similarly, if there's a reproduction buried in a comment, please move the link into the description and comment back. Please tag @brophdawg11 or @brookslybrand in your comment so we get a notification as well 🙂.
If this issue did not have a reproduction but is still valid, or if you wish to start with a fresh issue, please create a new issue with a fresh reproduction against v7 and link to this issue in the new description.
If this is a feature request, please open a new Proposal Discussion in React Router, and if it gets enough community support, it can be considered for implementation.
If you have any questions, you can always reach out on Discord. Thanks again for providing feedback and helping us make React Router even better!