From e7f79adc46b1dae2b92256187fd0ddd13eadc5fd Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Tue, 12 May 2026 08:40:25 +0200 Subject: [PATCH 1/3] meta(ai): add form submit + mutation guidance Two patterns kept getting flagged in form-migration PR reviews but were not explicit in the migrate-frontend-forms skill: - No generics on `useMutation`. Type the `mutationFn` payload and use `fetchMutation` for the return type. - Forms with a Save button must run the mutation inside `onSubmit` and use ``. A `` that is never submitted bypasses validation, pending/disabled state, and field-error wiring. Add a prominent "Patterns to Always Follow" section after the feature mapping table and matching items to the migration checklist. Refs https://github.com/getsentry/sentry/pull/115256#discussion_r3219679088 Refs https://github.com/getsentry/sentry/pull/115256#discussion_r3219655544 Co-Authored-By: Claude Opus 4.7 --- .../skills/migrate-frontend-forms/SKILL.md | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/.agents/skills/migrate-frontend-forms/SKILL.md b/.agents/skills/migrate-frontend-forms/SKILL.md index aa1a7ade4a1768..8906dd89042a5a 100644 --- a/.agents/skills/migrate-frontend-forms/SKILL.md +++ b/.agents/skills/migrate-frontend-forms/SKILL.md @@ -27,6 +27,76 @@ This skill helps migrate forms from Sentry's legacy form system (JsonForm, FormM | `label` | `label` | On layout components | | `required` | `required` | On layout + Zod schema | +## Patterns to Always Follow + +These are easy to miss and consistently flagged in review. + +### Don't pass generics to `useMutation` + +Type the `mutationFn` payload explicitly and let `fetchMutation` carry the return type. Putting generics on `useMutation` is not the pattern used in this codebase. + +```tsx +// ❌ Don't put generics on useMutation +const mutation = useMutation({ + mutationFn: ([payload]) => fetchMutation({url, method: 'POST', data: payload}), +}); + +// ✅ Type the payload on mutationFn; let fetchMutation carry the return type +const mutation = useMutation({ + mutationFn: (payload: {codeMappingId: string; raw: string}) => + fetchMutation({ + url: `/projects/${org}/${project}/codeowners/`, + method: 'POST', + data: payload, + }), +}); +``` + +### Submit through the form, not around it + +If a form has a Save button, wire it through `` and run the mutation in the form's `onSubmit`. Do **not** render `` just for layout/validation and trigger the mutation from a standalone ` + +); + +// ✅ Mutation runs in onSubmit; submit is form.SubmitButton +const form = useScrapsForm({ + ...defaultFormOptions, + defaultValues, + validators: {onDynamic: schema}, + onSubmit: ({value}) => + mutation + .mutateAsync(schema.parse(value)) + .then(data => { + onSave?.(data); + closeModal(); + }) + .catch(() => {}), +}); + +return ( + + {...} + Save + +); +``` + +The form's submit lifecycle handles validation, pending state, disabled state, and field-error wiring. Bypassing it with a standalone button means re-implementing all of that — and usually missing some. + ## Feature Details ### confirm → `confirm` prop @@ -602,6 +672,8 @@ This pattern is necessary whenever a required field has no meaningful initial va ## Migration Checklist - [ ] Replace JsonForm/FormModel with useScrapsForm or AutoSaveForm +- [ ] No generics on `useMutation` — type the `mutationFn` payload and use `fetchMutation` for the return type +- [ ] When using `useScrapsForm` with a Save button: mutation runs in `onSubmit`, triggered by `` (no form that's never submitted) - [ ] Convert field config objects to JSX AppField components - [ ] Replace `help` → `hintText` on layouts - [ ] Replace `showHelpInTooltip` → `variant="compact"` From a055c2c58e23f13889c2675766d414ec10ee9ec1 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Tue, 12 May 2026 08:54:56 +0200 Subject: [PATCH 2/3] meta(ai): inline new guidance into existing sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restructure the previous commit so the two notes follow the skill's existing pattern (guidance embedded under the relevant feature subsection) instead of a new top-level section: - Move the `useMutation` generics note next to the existing "typing mutations correctly" callout under `getData → mutationFn`. - Move the "submit through the form" note under `saveOnBlur: false → useScrapsForm`, which is where the Save button pattern lives. The migration checklist entries stay — the checklist already aggregates points from across sections. Co-Authored-By: Claude Opus 4.7 --- .../skills/migrate-frontend-forms/SKILL.md | 130 ++++++++---------- 1 file changed, 60 insertions(+), 70 deletions(-) diff --git a/.agents/skills/migrate-frontend-forms/SKILL.md b/.agents/skills/migrate-frontend-forms/SKILL.md index 8906dd89042a5a..85e5093cd76021 100644 --- a/.agents/skills/migrate-frontend-forms/SKILL.md +++ b/.agents/skills/migrate-frontend-forms/SKILL.md @@ -27,76 +27,6 @@ This skill helps migrate forms from Sentry's legacy form system (JsonForm, FormM | `label` | `label` | On layout components | | `required` | `required` | On layout + Zod schema | -## Patterns to Always Follow - -These are easy to miss and consistently flagged in review. - -### Don't pass generics to `useMutation` - -Type the `mutationFn` payload explicitly and let `fetchMutation` carry the return type. Putting generics on `useMutation` is not the pattern used in this codebase. - -```tsx -// ❌ Don't put generics on useMutation -const mutation = useMutation({ - mutationFn: ([payload]) => fetchMutation({url, method: 'POST', data: payload}), -}); - -// ✅ Type the payload on mutationFn; let fetchMutation carry the return type -const mutation = useMutation({ - mutationFn: (payload: {codeMappingId: string; raw: string}) => - fetchMutation({ - url: `/projects/${org}/${project}/codeowners/`, - method: 'POST', - data: payload, - }), -}); -``` - -### Submit through the form, not around it - -If a form has a Save button, wire it through `` and run the mutation in the form's `onSubmit`. Do **not** render `` just for layout/validation and trigger the mutation from a standalone ` - -); - -// ✅ Mutation runs in onSubmit; submit is form.SubmitButton -const form = useScrapsForm({ - ...defaultFormOptions, - defaultValues, - validators: {onDynamic: schema}, - onSubmit: ({value}) => - mutation - .mutateAsync(schema.parse(value)) - .then(data => { - onSave?.(data); - closeModal(); - }) - .catch(() => {}), -}); - -return ( - - {...} - Save - -); -``` - -The form's submit lifecycle handles validation, pending state, disabled state, and field-error wiring. Bypassing it with a standalone button means re-implementing all of that — and usually missing some. - ## Feature Details ### confirm → `confirm` prop @@ -286,6 +216,25 @@ mutationOptions={{ Make sure the zod schema's types are compatible with (i.e., assignable to) the API type. For example, if the API expects a string union like `'off' | 'low' | 'high'`, use `z.enum(['off', 'low', 'high'])` instead of `z.string()`. +**Don't pass generics to `useMutation` either.** Type the `mutationFn` payload, and let `fetchMutation` carry the return type. `useMutation` is not the codebase style. + +```tsx +// ❌ Generics on useMutation +const mutation = useMutation({ + mutationFn: ([payload]) => fetchMutation({url, method: 'POST', data: payload}), +}); + +// ✅ Type the payload; fetchMutation carries the return type +const mutation = useMutation({ + mutationFn: (payload: {codeMappingId: string; raw: string}) => + fetchMutation({ + url: `/projects/${org}/${project}/codeowners/`, + method: 'POST', + data: payload, + }), +}); +``` + ### mapFormErrors → `setFieldErrors` The `mapFormErrors` function transformed API error responses into field-specific errors. In the new system, handle this in the catch block using `setFieldErrors`. @@ -567,6 +516,47 @@ function SlugForm({project}: {project: Project}) { - Large multiline text fields where you want to finish editing before saving (fingerprint rules, filters) - Any field where auto-save doesn't make sense +**Submit through the form, not around it.** The mutation must run in the form's `onSubmit` and the Save button must be ``. Don't render `` just for layout/validation and trigger the mutation from a standalone ` + +); + +// ✅ Mutation runs in onSubmit; Save is form.SubmitButton +const form = useScrapsForm({ + ...defaultFormOptions, + defaultValues, + validators: {onDynamic: schema}, + onSubmit: ({value}) => + mutation + .mutateAsync(schema.parse(value)) + .then(data => { + onSave?.(data); + closeModal(); + }) + .catch(() => {}), +}); + +return ( + + {...} + Save + +); +``` + ## Preserving Form Search Functionality Sentry's SettingsSearch allows users to search for individual settings fields. When migrating forms, you must preserve this searchability by wrapping migrated forms with `FormSearch`. From 5419a5393fa9fe148421b9b6bee0a35628ead4f8 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Tue, 12 May 2026 09:02:21 +0200 Subject: [PATCH 3/3] meta(ai): trim duplicate submit example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ✅ side of the submit block restated what the SlugForm example above already shows. Keep only the ❌ antipattern and point readers back to SlugForm for the correct shape. Co-Authored-By: Claude Opus 4.7 --- .../skills/migrate-frontend-forms/SKILL.md | 26 +++---------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/.agents/skills/migrate-frontend-forms/SKILL.md b/.agents/skills/migrate-frontend-forms/SKILL.md index 85e5093cd76021..04af03de3ed617 100644 --- a/.agents/skills/migrate-frontend-forms/SKILL.md +++ b/.agents/skills/migrate-frontend-forms/SKILL.md @@ -516,7 +516,7 @@ function SlugForm({project}: {project: Project}) { - Large multiline text fields where you want to finish editing before saving (fingerprint rules, filters) - Any field where auto-save doesn't make sense -**Submit through the form, not around it.** The mutation must run in the form's `onSubmit` and the Save button must be ``. Don't render `` just for layout/validation and trigger the mutation from a standalone ` ); - -// ✅ Mutation runs in onSubmit; Save is form.SubmitButton -const form = useScrapsForm({ - ...defaultFormOptions, - defaultValues, - validators: {onDynamic: schema}, - onSubmit: ({value}) => - mutation - .mutateAsync(schema.parse(value)) - .then(data => { - onSave?.(data); - closeModal(); - }) - .catch(() => {}), -}); - -return ( - - {...} - Save - -); ``` +A form that's never actually submitted bypasses validation, pending/disabled state, and field-error wiring. + ## Preserving Form Search Functionality Sentry's SettingsSearch allows users to search for individual settings fields. When migrating forms, you must preserve this searchability by wrapping migrated forms with `FormSearch`.