-
Notifications
You must be signed in to change notification settings - Fork 25
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
useMutationForm hook #2301
useMutationForm hook #2301
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,43 +1,25 @@ | ||
import { FC, useMemo, useState } from "react"; | ||
import { FormProvider, useFieldArray, useForm } from "react-hook-form"; | ||
import { FormProvider, useFieldArray } from "react-hook-form"; | ||
import { graphql, useFragment } from "react-relay"; | ||
|
||
import { PlaygroundToolbarItem } from "@quri/squiggle-components"; | ||
import { Button, LinkIcon, TextTooltip, useToast } from "@quri/ui"; | ||
import { Button, LinkIcon, TextTooltip } from "@quri/ui"; | ||
import { | ||
SquigglePlaygroundVersionPicker, | ||
SquiggleVersionShower, | ||
VersionedSquigglePlayground, | ||
type SquiggleVersion, | ||
} from "@quri/versioned-playground"; | ||
|
||
import { EditSquiggleSnippetModel$key } from "@/__generated__/EditSquiggleSnippetModel.graphql"; | ||
import { | ||
EditSquiggleSnippetModelMutation, | ||
RelativeValuesExportInput, | ||
} from "@/__generated__/EditSquiggleSnippetModelMutation.graphql"; | ||
import { EditModelExports } from "@/components/exports/EditModelExports"; | ||
import { useAsyncMutation } from "@/hooks/useAsyncMutation"; | ||
import { useAvailableHeight } from "@/hooks/useAvailableHeight"; | ||
import { useMutationForm } from "@/hooks/useMutationForm"; | ||
import { extractFromGraphqlErrorUnion } from "@/lib/graphqlHelpers"; | ||
import { | ||
SquigglePlaygroundVersionPicker, | ||
VersionedSquigglePlayground, | ||
type SquiggleVersion, | ||
SquiggleVersionShower, | ||
} from "@quri/versioned-playground"; | ||
|
||
export const Mutation = graphql` | ||
mutation EditSquiggleSnippetModelMutation( | ||
$input: MutationUpdateSquiggleSnippetModelInput! | ||
) { | ||
result: updateSquiggleSnippetModel(input: $input) { | ||
__typename | ||
... on BaseError { | ||
message | ||
} | ||
... on UpdateSquiggleSnippetResult { | ||
model { | ||
...EditSquiggleSnippetModel | ||
} | ||
} | ||
} | ||
} | ||
`; | ||
|
||
type FormShape = { | ||
code: string; | ||
|
@@ -51,8 +33,6 @@ type Props = { | |
}; | ||
|
||
export const EditSquiggleSnippetModel: FC<Props> = ({ modelRef }) => { | ||
const toast = useToast(); | ||
|
||
const model = useFragment( | ||
graphql` | ||
fragment EditSquiggleSnippetModel on Model { | ||
|
@@ -96,8 +76,6 @@ export const EditSquiggleSnippetModel: FC<Props> = ({ modelRef }) => { | |
"SquiggleSnippet" | ||
); | ||
|
||
const { height, ref } = useAvailableHeight(); | ||
|
||
const initialFormValues: FormShape = useMemo(() => { | ||
return { | ||
code: content.code, | ||
|
@@ -111,8 +89,42 @@ export const EditSquiggleSnippetModel: FC<Props> = ({ modelRef }) => { | |
}; | ||
}, [content, revision.relativeValuesExports]); | ||
|
||
const form = useForm<FormShape>({ | ||
const { form, onSubmit, inFlight } = useMutationForm< | ||
FormShape, | ||
EditSquiggleSnippetModelMutation, | ||
"UpdateSquiggleSnippetResult" | ||
>({ | ||
defaultValues: initialFormValues, | ||
mutation: graphql` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still not sure if inlining mutations is good or bad, but started to lean towards it being good. Mutation name ( So if the mutation was declared separately outside of the component, you'd have to jump back and forth to change it. |
||
mutation EditSquiggleSnippetModelMutation( | ||
$input: MutationUpdateSquiggleSnippetModelInput! | ||
) { | ||
result: updateSquiggleSnippetModel(input: $input) { | ||
__typename | ||
... on BaseError { | ||
message | ||
} | ||
... on UpdateSquiggleSnippetResult { | ||
model { | ||
...EditSquiggleSnippetModel | ||
} | ||
} | ||
} | ||
} | ||
`, | ||
expectedTypename: "UpdateSquiggleSnippetResult", | ||
formDataToVariables: (formData) => ({ | ||
input: { | ||
content: { | ||
code: formData.code, | ||
version, | ||
}, | ||
relativeValuesExports: formData.relativeValuesExports, | ||
slug: model.slug, | ||
owner: model.owner.slug, | ||
}, | ||
}), | ||
confirmation: "Saved", | ||
}); | ||
|
||
// could version picker be part of the form? | ||
|
@@ -127,31 +139,6 @@ export const EditSquiggleSnippetModel: FC<Props> = ({ modelRef }) => { | |
control: form.control, | ||
}); | ||
|
||
const [saveMutation, saveInFlight] = useAsyncMutation< | ||
EditSquiggleSnippetModelMutation, | ||
"UpdateSquiggleSnippetResult" | ||
>({ | ||
mutation: Mutation, | ||
expectedTypename: "UpdateSquiggleSnippetResult", | ||
}); | ||
|
||
const save = form.handleSubmit((formData) => { | ||
saveMutation({ | ||
variables: { | ||
input: { | ||
content: { | ||
code: formData.code, | ||
version, | ||
}, | ||
relativeValuesExports: formData.relativeValuesExports, | ||
slug: model.slug, | ||
owner: model.owner.slug, | ||
}, | ||
}, | ||
onCompleted: () => toast("Saved", "confirmation"), | ||
}); | ||
}); | ||
|
||
const onCodeChange = (code: string) => { | ||
form.setValue("code", code); | ||
}; | ||
|
@@ -165,9 +152,11 @@ export const EditSquiggleSnippetModel: FC<Props> = ({ modelRef }) => { | |
setDefaultCode(form.getValues("code")); | ||
}; | ||
|
||
const { height, ref } = useAvailableHeight(); | ||
|
||
return ( | ||
<FormProvider {...form}> | ||
<form onSubmit={save}> | ||
<form onSubmit={onSubmit}> | ||
<div ref={ref}> | ||
<VersionedSquigglePlayground | ||
version={version} | ||
|
@@ -205,9 +194,9 @@ export const EditSquiggleSnippetModel: FC<Props> = ({ modelRef }) => { | |
{model.isEditable && ( | ||
<Button | ||
theme="primary" | ||
onClick={save} | ||
onClick={onSubmit} | ||
size="small" | ||
disabled={saveInFlight} | ||
disabled={inFlight} | ||
> | ||
Save | ||
</Button> | ||
|
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.
This duplicates the
expectedTypename
field, and unfortunately it's necessary for type safety. (It's also necessary inuseAsyncMutation
that we had before, but this hook's type parameters are stricter).This is a known TypeScript limitation: it lacks partial type argument inference (see microsoft/TypeScript#26242 for the details).
I hope they'll get around to adding it in the next 6-12 months (people have been complaining about its priority for a long time, e.g. microsoft/TypeScript#55486 (comment)), but until then we'll have to tolerate it.