-
Notifications
You must be signed in to change notification settings - Fork 107
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
Replace unknown
in submission payload
#706
Conversation
🦋 Changeset detectedLatest commit: e44c9e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
7b3b246
to
1c82e88
Compare
This does not seem to work for some playground files because Let me know if that works and I will change this PR @edmundhung |
Thanks for the PR @timvandam. I was not aware that I can represent the submission payload this way, but now I see you did that it make total sense 👍
We are stripping all files already if you are running on the server. So to make it non-breaking, we will need to default the new option to I can't push any changes to your PR likely because your PR was based on your main branch. This is what I am thinking: export type SubmissionPayload<Entry extends FormDataEntryValue> =
| Entry
| SubmissionPayload<Entry>[]
| { [key: string]: SubmissionPayload<Entry> };
export type Submission<Schema, FormError = string[], FormValue = Schema> =
| {
status: 'success';
payload: Record<string, SubmissionPayload<FormDataEntryValue>>;
value: FormValue;
reply(options?: ReplyOptions<FormError>): SubmissionResult<FormError>;
}
| {
status: 'error' | undefined;
payload: Record<string, SubmissionPayload<FormDataEntryValue>>;
error: Record<string, FormError | null> | null;
reply(options?: ReplyOptions<FormError>): SubmissionResult<FormError>;
};
export type SubmissionResult<FormError = string[]> = {
status?: 'error' | 'success';
intent?: Intent;
initialValue?: Record<string, SubmissionPayload<string>> | null;
fields?: string[];
error?: Record<string, FormError | null>;
state?: SubmissionState;
}; Then, on const initialValue = normalize(
context.payload,
// We can't serialize the file and send it back from the server, but we can preserve it in the client
typeof document !== 'undefined',
// We need the file on the client because it's treated as the form value
// But we will exclude the File type for now as it's only used by the internal
// form state and we will remove the need to preserve the file on the client soon
) as Record<string, SubmissionPayload<string>> ?? {}
return {
initialValue,
// ...
}; |
615031b
to
33d1223
Compare
Awesome, not sure why you cannot push to my branch, but I've added what you've commented. Very nice solution with the generic, I was thinking of much more complex ways of going about it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this change. Thanks!
More templates
@conform-to/dom
@conform-to/react
@conform-to/validitystate
@conform-to/zod
@conform-to/yup
commit: |
Hi guys, when will this PR be released, this is very much needed right now for Remix and Single Fetch? |
I am hoping to cut a release sometimes this month. If you can give the pre-release version a try and let me know whether you run into any issues, it would help me releasing this change sooner. Thanks! |
Sorry to sound dim, how do I do this, do I just change my package .json to ...
|
There is a comment above from pkg.pr.new with each package name listed. You will find a command to install the pre-release version of that package if you expand the item. If you are not using pnpm, you can swap it with npm install, or just copy the package URL and replace the version in the package.json with it. Really appreciate for giving this a try! 👍 |
Fix #628
This pull request replaces the
unknown
type in submission payloads.unknown
is not compatible with Remix Single Fetch and newdefineLoader
ordefineAction
as it cannot be guaranteed that values of typeunknown
can be sent over the wire.Removing
unknown
types addresses this issue. I've replaced it with what I think is correct but I don't know the ins and outs of this library so plz check.Thanks!