diff --git a/.config/mise/conf.d/_config.toml b/.config/mise/conf.d/_config.toml index 43174020..835da22d 100644 --- a/.config/mise/conf.d/_config.toml +++ b/.config/mise/conf.d/_config.toml @@ -5,7 +5,7 @@ min_version = "2026.4.18" # required for [deps] (renamed from [prepare]) node = "lts" jq = "latest" vp = "latest" -hk = "latest" +hk = "1" # hk 2 cannot evaluate a config that amends the 1.x Config.pkl [task_config] dir = "{{ config_root }}" diff --git a/apps/demo/messages/en/articles.json b/apps/demo/messages/en/articles.json index 3724e364..bf34b443 100644 --- a/apps/demo/messages/en/articles.json +++ b/apps/demo/messages/en/articles.json @@ -15,7 +15,6 @@ "article_search_placeholder": "Search articles...", "article_loading_detail": "Loading article detail", "article_saving": "Saving…", - "article_saved": "Article saved", "article_save_error": "Couldn't save the article. Try again.", "article_cancel": "Cancel", "article_save": "Save", diff --git a/apps/demo/messages/en/auth.json b/apps/demo/messages/en/auth.json index adf57aba..2ddd232c 100644 --- a/apps/demo/messages/en/auth.json +++ b/apps/demo/messages/en/auth.json @@ -3,6 +3,9 @@ "login_description": "Use your workspace credentials to continue.", "login_email": "Email", "login_password": "Password", + "login_email_required": "Email is required", + "login_email_invalid": "Email doesn't look right", + "login_password_required": "Password is required", "login_submit": "Sign in", "login_signing_in": "Signing in", "login_error_title": "Could not sign in", diff --git a/apps/demo/messages/en/settings.json b/apps/demo/messages/en/settings.json index 93df247f..3493a66e 100644 --- a/apps/demo/messages/en/settings.json +++ b/apps/demo/messages/en/settings.json @@ -42,7 +42,5 @@ "settings_error_title": "Could not load settings", "settings_error_description": "We couldn't load your settings. Try again in a moment.", "settings_saving": "Saving…", - "settings_profile_saved": "Profile saved", - "settings_notifications_saved": "Notification preferences saved", "settings_save_error": "Couldn't save. Try again." } diff --git a/apps/demo/messages/es/articles.json b/apps/demo/messages/es/articles.json index 23c64d1d..7c0b36fe 100644 --- a/apps/demo/messages/es/articles.json +++ b/apps/demo/messages/es/articles.json @@ -15,7 +15,6 @@ "article_search_placeholder": "Buscar artículos...", "article_loading_detail": "Cargando detalle del artículo", "article_saving": "Guardando…", - "article_saved": "Artículo guardado", "article_save_error": "No se pudo guardar el artículo. Inténtalo de nuevo.", "article_cancel": "Cancelar", "article_save": "Guardar", diff --git a/apps/demo/messages/es/auth.json b/apps/demo/messages/es/auth.json index cf861411..718b8a59 100644 --- a/apps/demo/messages/es/auth.json +++ b/apps/demo/messages/es/auth.json @@ -3,6 +3,9 @@ "login_description": "Usa tus credenciales del espacio de trabajo para continuar.", "login_email": "Correo electrónico", "login_password": "Contraseña", + "login_email_required": "El correo electrónico es obligatorio", + "login_email_invalid": "El correo no parece válido", + "login_password_required": "La contraseña es obligatoria", "login_submit": "Iniciar sesión", "login_signing_in": "Iniciando sesión", "login_error_title": "No se pudo iniciar sesión", diff --git a/apps/demo/messages/es/settings.json b/apps/demo/messages/es/settings.json index 6a263f91..e7a421e3 100644 --- a/apps/demo/messages/es/settings.json +++ b/apps/demo/messages/es/settings.json @@ -42,7 +42,5 @@ "settings_error_title": "No se pudieron cargar los ajustes", "settings_error_description": "No pudimos cargar tus ajustes. Inténtalo de nuevo en un momento.", "settings_saving": "Guardando…", - "settings_profile_saved": "Perfil guardado", - "settings_notifications_saved": "Preferencias de notificaciones guardadas", "settings_save_error": "No se pudo guardar. Inténtalo de nuevo." } diff --git a/apps/demo/src/app/integration/Articles.detail.stories.tsx b/apps/demo/src/app/integration/Articles.detail.stories.tsx index f1b395ce..593484b3 100644 --- a/apps/demo/src/app/integration/Articles.detail.stories.tsx +++ b/apps/demo/src/app/integration/Articles.detail.stories.tsx @@ -174,7 +174,7 @@ EditArticle.test('saving updates the title and returns to read mode', async () = await I.openEdit() await I.fill(role('textbox', 'Title'), 'Updated quarterly report') await I.saveArticle() - await I.seeArticleSavedToast() + await I.seeArticleSaved() await I.see(button('Edit')) await I.see(heading('Updated quarterly report')) }) @@ -185,11 +185,9 @@ EditArticle.test('changing status persists and reflects on return', async () => await I.openEdit() await I.selectOption(role('combobox'), 'In Progress') await I.saveArticle() - await I.seeArticleSavedToast() - // A prior save test in this story leaves a stale "Article saved" toast - // in the global toaster, so `seeArticleSavedToast` can resolve against it - // before this save transitions to read mode. Wait for the real transition. - await I.retryTo(() => I.see(button('Edit')), 25) + // The edit panel collapsing to the summary rows is what confirms the + // save — there is no toast to observe. + await I.seeArticleSaved() await I.seeArticleDetailStatus('In Progress') }) }) @@ -204,13 +202,13 @@ export const EditArticleServerError = meta.story({ }) EditArticleServerError.test( - 'save failure shows an error toast and stays in edit mode', + 'save failure shows an inline alert and stays in edit mode', async () => { await I.scope(role('main'), async () => { await I.openEdit() await I.fill(role('textbox', 'Title'), 'Will not save') await I.saveArticle() - await I.seeArticleSaveErrorToast() + await I.seeArticleSaveError() await I.see(role('textbox', 'Title')) }) }, diff --git a/apps/demo/src/app/integration/Settings.stories.tsx b/apps/demo/src/app/integration/Settings.stories.tsx index 3fd02ed9..50f46764 100644 --- a/apps/demo/src/app/integration/Settings.stories.tsx +++ b/apps/demo/src/app/integration/Settings.stories.tsx @@ -92,8 +92,7 @@ EditProfileShowsSave.test('save button disappears after saving', async () => { await I.fill(role('textbox', 'Display name'), 'Jane Doe') await I.see(button('Save changes')) await I.saveProfile() - await I.seeProfileSavedToast() - await I.dontSee(button('Save changes')) + await I.savedProfileClearsDirty(loc.profileForm) }) export const ToggleSwitches = meta.story({ name: 'Toggle Switches', play: waitForLoad }) @@ -246,12 +245,11 @@ RetrySuccess.test('loads settings after retry succeeds', async () => { export const SaveProfileSuccess = meta.story({ name: 'Save Profile', play: waitForLoad }) -SaveProfileSuccess.test('saving profile shows success toast and clears dirty', async () => { +SaveProfileSuccess.test('saving profile clears dirty and keeps the saved values', async () => { await I.fill(role('textbox', 'Display name'), 'Jane Doe') await I.see(button('Save changes')) await I.saveProfile() - await I.seeProfileSavedToast() - await I.dontSee(button('Save changes')) + await I.savedProfileClearsDirty(loc.profileForm) await I.seeInField(role('textbox', 'Display name'), 'Jane Doe') }) @@ -260,16 +258,12 @@ export const SaveNotificationsSuccess = meta.story({ play: waitForLoad, }) -SaveNotificationsSuccess.test( - 'saving notifications shows success toast and clears dirty', - async () => { - await I.selectOption(role('combobox', 'Email notifications'), 'Important only') - await I.see(button('Save changes')) - await I.saveNotifications() - await I.seeNotificationsSavedToast() - await I.dontSee(button('Save changes')) - }, -) +SaveNotificationsSuccess.test('saving notifications clears dirty', async () => { + await I.selectOption(role('combobox', 'Email notifications'), 'Important only') + await I.see(button('Save changes')) + await I.saveNotifications() + await I.savedProfileClearsDirty(loc.notificationsForm) +}) export const SaveProfileError = meta.story({ name: 'Save Profile Error', @@ -280,9 +274,9 @@ export const SaveProfileError = meta.story({ }, }) -SaveProfileError.test('save server error shows error toast and keeps dirty', async () => { +SaveProfileError.test('save server error shows an inline alert and keeps dirty', async () => { await I.fill(role('textbox', 'Display name'), 'Jane Doe') await I.saveProfile() - await I.seeSaveErrorToast() + await I.seeSaveError(loc.profileForm) await I.see(button('Save changes')) }) diff --git a/apps/demo/src/entities/auth/mocks/handlers.ts b/apps/demo/src/entities/auth/mocks/handlers.ts index e7f141f1..7e90d5b5 100644 --- a/apps/demo/src/entities/auth/mocks/handlers.ts +++ b/apps/demo/src/entities/auth/mocks/handlers.ts @@ -2,7 +2,7 @@ import { HttpResponse, delay, http } from 'msw' import { AUTH_LOGIN_API_PATH, AUTH_LOGOUT_API_PATH } from '#entities/auth/api/authApi' import { composeApiUrl } from '#shared/api' -import { to400, to500 } from '#shared/mocks/utils' +import { to400, to422, to500 } from '#shared/mocks/utils' import { authMockSession } from './data' @@ -13,6 +13,12 @@ export const authHandlers = { login: http.post(loginUrl, async ({ request }) => { await delay() const body = (await request.json()) as { email?: string; password?: string } + // A server-side uniqueness check is the one credential rule the client + // cannot pre-validate — it demonstrates a validation error that maps + // onto a form field. + if (body.email === 'taken@example.com') { + to422([{ loc: ['body', 'email'], msg: 'This email is already registered' }]) + } if (body.email !== authMockSession.user.email || body.password !== 'password') { return to400('Invalid email or password') } diff --git a/apps/demo/src/entities/setting/index.ts b/apps/demo/src/entities/setting/index.ts index b1616572..a09baaa7 100644 --- a/apps/demo/src/entities/setting/index.ts +++ b/apps/demo/src/entities/setting/index.ts @@ -1,8 +1,2 @@ export { fetchSettings, updateNotifications, updateProfile } from './api/settingsApi' -export type { - DesktopNotification, - EmailNotification, - NotificationSettings, - ProfileSettings, - SettingsData, -} from './model/types' +export type { DesktopNotification, EmailNotification, SettingsData } from './model/types' diff --git a/apps/demo/src/pages/articles/model/articleDetailModel.test.ts b/apps/demo/src/pages/articles/model/articleDetailModel.test.ts new file mode 100644 index 00000000..720335e7 --- /dev/null +++ b/apps/demo/src/pages/articles/model/articleDetailModel.test.ts @@ -0,0 +1,67 @@ +import type { Article } from '#entities/article' + +import { context } from '@reatom/core' +import { afterEach, expect, test, vi } from 'vite-plus/test' + +import { updateArticle } from '#entities/article' + +import { reatomArticleDetailModel } from './articleDetailModel' + +vi.mock('#entities/article', () => ({ updateArticle: vi.fn() })) + +afterEach(() => { + context.reset() + vi.mocked(updateArticle).mockReset() +}) + +// `submit.onFulfill` hooks run in the next cleanup-queue tick. +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)) + +const article: Article = { + id: 'a1', + title: 'Original', + description: 'Description', + status: 'draft', + content: [], +} + +const respondWith = (release: Promise) => + vi.mocked(updateArticle).mockImplementation(async (id, values) => { + await release + return { id, ...values } + }) + +test('a save with nothing typed during the request closes edit mode', async () => { + respondWith(Promise.resolve()) + const model = reatomArticleDetailModel(article) + model.startEdit() + + model.form.fields.title.change('Saved') + await model.form.submit() + await flush() + + expect(model.current().title).toBe('Saved') + expect(model.isEditing()).toBe(false) +}) + +test('an edit typed while the save is in flight keeps edit mode open', async () => { + let release: (() => void) | undefined + respondWith( + new Promise((resolve) => { + release = resolve + }), + ) + const model = reatomArticleDetailModel(article) + model.startEdit() + + model.form.fields.title.change('Sent') + const submitted = model.form.submit() + model.form.fields.title.change('Typed during save') + release?.() + await submitted + await flush() + + expect(model.current().title).toBe('Sent') + expect(model.form.fields.title.value()).toBe('Typed during save') + expect(model.isEditing()).toBe(true) +}) diff --git a/apps/demo/src/pages/articles/model/articleDetailModel.ts b/apps/demo/src/pages/articles/model/articleDetailModel.ts index 3d58452f..a067fd1c 100644 --- a/apps/demo/src/pages/articles/model/articleDetailModel.ts +++ b/apps/demo/src/pages/articles/model/articleDetailModel.ts @@ -1,38 +1,9 @@ import type { Article } from '#entities/article' -import { - abortVar, - action, - atom, - framePromise, - reatomForm, - sleep, - withAbort, - withAsync, - wrap, -} from '@reatom/core' +import { abortVar, action, atom, reatomForm, wrap } from '@reatom/core' import { updateArticle } from '#entities/article' -import { m } from '#paraglide/messages.js' -import { toaster } from '#shared/components' - -const SAVE_DELAY_MS = 300 - -async function saveWithToast(articleId: string, values: Omit) { - const id = toaster.create({ title: m.article_saving(), type: 'loading', closable: false }) - void framePromise().catch(() => {}) - try { - const updated = await wrap( - updateArticle(articleId, values, { signal: abortVar.require().signal }), - ) - await wrap(sleep(SAVE_DELAY_MS)) - toaster.update(id, { title: m.article_saved(), type: 'success' }) - return updated - } catch (error) { - toaster.remove(id) - throw error - } -} +import { withSavedState } from '#shared/reatom' export function reatomArticleDetailModel(article: Article) { const id = article.id @@ -48,8 +19,27 @@ export function reatomArticleDetailModel(article: Article) { }, { name: `article.${id}.editForm`, - onSubmit: async (values) => await wrap(saveWithToast(id, values)), + onSubmit: async (values) => { + const updated = await wrap(updateArticle(id, values, { signal: abortVar.require().signal })) + // The summary rows move when the request lands; withSavedState then + // rebaselines the form and its onSaved collapses back to them — + // seeing the new values on the rows is what confirms the save. + current.set(updated) + // withSavedState rebaselines through form.init, whose keys must be + // field names exactly: returning the full Article (with `id`) would + // throw `Field id not found in fields`. + const { id: _, ...fields } = updated + return fields + }, }, + ).extend((target) => + withSavedState({ + // An edit typed while the request was in flight keeps the form dirty; + // collapsing would hide it behind rows showing the saved values. + onSaved: () => { + if (!target.focus().dirty) isEditing.set(false) + }, + })(target), ) const startEdit = action(() => { @@ -62,24 +52,7 @@ export function reatomArticleDetailModel(article: Article) { isEditing.set(true) }, `article.${id}.startEdit`) - const save = action(async () => { - if (!form.focus().dirty) return - try { - const updated = await wrap(form.submit()) - current.set(updated) - form.init({ - title: updated.title, - description: updated.description, - status: updated.status, - content: updated.content, - }) - isEditing.set(false) - } catch { - toaster.create({ title: m.article_save_error(), type: 'error' }) - } - }, `article.${id}.save`).extend(withAbort(), withAsync()) - - return { id, current, isEditing, form, startEdit, save } + return { id, current, isEditing, form, startEdit } } export type ArticleDetailModel = ReturnType diff --git a/apps/demo/src/pages/articles/testing.ts b/apps/demo/src/pages/articles/testing.ts index 0c7e1308..2d388f49 100644 --- a/apps/demo/src/pages/articles/testing.ts +++ b/apps/demo/src/pages/articles/testing.ts @@ -20,6 +20,7 @@ const ARTICLE_LINKS = [ ] as const const editLoc = { + editForm: role('form', m.article_detail()), editButton: button('Edit'), saveButton: button('Save'), cancelButton: button('Cancel'), @@ -130,17 +131,13 @@ export const articlesActor = createActor() saveArticle: async () => { await I.click(editLoc.saveButton) }, - seeArticleSavedToast: async () => { - // Require the current save's loading toast before accepting success, so a - // stale global success toast from another story/test cannot satisfy this helper. - await I.retryTo(() => I.see(role('status', 'Saving…').within('global')), 25) - await I.retryTo(() => I.see(role('status', 'Article saved').within('global')), 25) - }, - seeArticleSaveErrorToast: async () => { - await I.retryTo( - () => I.see(text("Couldn't save the article. Try again.").within('global')), - 25, - ) + // A save is confirmed by the state change itself: the edit panel collapses + // to the summary rows, which now state the saved values — there is no toast. + seeArticleSaved: async () => { + await I.retryTo(() => I.see(editLoc.editButton), 25) + }, + seeArticleSaveError: async () => { + await I.retryTo(() => I.see(role('alert').within(editLoc.editForm)), 25) }, seeTitleIs: async (value: string) => { await I.retryTo(() => I.seeInField(editLoc.titleField, value), 25) diff --git a/apps/demo/src/pages/articles/ui/detail/ArticleDetail.tsx b/apps/demo/src/pages/articles/ui/detail/ArticleDetail.tsx index 653b286d..37c86809 100644 --- a/apps/demo/src/pages/articles/ui/detail/ArticleDetail.tsx +++ b/apps/demo/src/pages/articles/ui/detail/ArticleDetail.tsx @@ -6,8 +6,9 @@ import { wrap } from '@reatom/core' import { bindField, reatomComponent } from '@reatom/react' import { m } from '#paraglide/messages.js' -import { Button, CollectionSelect, Heading, Input, Text } from '#shared/components' +import { Alert, Button, CollectionSelect, Heading, Input, Text } from '#shared/components' import { reatomLoc } from '#shared/model' +import { formAlertMessage } from '#shared/reatom' import { styled } from '#styled-system/jsx' import { ArticleStatusBadge } from '../ArticleStatusBadge' @@ -29,15 +30,29 @@ const statusCollection = reatomLoc( export const ArticleDetail = reatomComponent(({ model }: { model: ArticleDetailModel }) => { if (model.isEditing()) { const isDirty = model.form.focus().dirty + // Save failures here are network-level — no field owns them — so the + // alert carries the whole failure and the edit stays on screen, dirty. + const showSaveError = formAlertMessage(model.form) !== null return ( { e.preventDefault() - model.save() + model.form.submit() })} > + {showSaveError && ( + + + + {m.article_save_error()} + + + )} {isDirty && ( + ) +}) + export const SettingsPage = reatomComponent(({ model }: { model: SettingsPageModel }) => { - const { profileForm, saveProfile, notificationsForm, saveNotifications, appearanceForm } = model + const { profileForm, notificationsForm, appearanceForm } = model + // Save failures are network-level — no field owns them — so the alert + // carries the whole failure and the fields stay untouched. + const showProfileError = formAlertMessage(profileForm) !== null + const showNotificationsError = formAlertMessage(notificationsForm) !== null return ( @@ -102,19 +131,16 @@ export const SettingsPage = reatomComponent(({ model }: { model: SettingsPageMod
saveProfile())} - > - {m.settings_save_changes()} - - ) : null - } + footer={ profileForm.submit())} />} > + {showProfileError && ( + + + + {m.settings_save_error()} + + + )} @@ -131,18 +157,17 @@ export const SettingsPage = reatomComponent(({ model }: { model: SettingsPageMod
saveNotifications())} - > - {m.settings_save_changes()} - - ) : null + notificationsForm.submit())} /> } > + {showNotificationsError && ( + + + + {m.settings_save_error()} + + + )} new Response(null, { status }) + +afterEach(() => { + vi.unstubAllGlobals() +}) + +// Every shape `extractErrorCode` understands. The naive version only reads +// `error.message`; real backends send FastAPI `detail` strings, FastAPI +// validation arrays, and envelope `{ error: { code } }` objects. +test('error codes across payload shapes', () => { + // detail: string is the code itself + expect(createApiError(response(403), { detail: 'FORBIDDEN' }).code).toBe('FORBIDDEN') + // detail: non-empty array means validation + expect(new ApiError(500, { detail: [{ msg: 'x' }] }).code).toBe('VALIDATION_ERROR') + // detail: empty array carries nothing + expect(new ApiError(500, { detail: [] }).code).toBeNull() + // detail: envelope object with a code + expect(new ApiError(500, { detail: { code: 'ORG_SUSPENDED' } }).code).toBe('ORG_SUSPENDED') + // detail: envelope object with a non-string code + expect(new ApiError(500, { detail: { code: 7 } }).code).toBeNull() + // error: string + expect(new ApiError(500, { error: 'SOMETHING_ELSE' }).code).toBe('SOMETHING_ELSE') + // error: envelope object with a message + expect(new ApiError(500, { error: { message: 'nope' } }).code).toBe('nope') + // error: envelope object with a non-string message + expect(new ApiError(500, { error: { message: 42 } }).code).toBeNull() + // error: non-empty array means validation + expect(new ApiError(500, { error: [{ msg: 'x' }] }).code).toBe('VALIDATION_ERROR') + // error: empty array carries nothing + expect(new ApiError(500, { error: [] }).code).toBeNull() + // an object with neither key carries nothing + expect(new ApiError(500, { status: 'down' }).code).toBeNull() +}) + +const jsonHttpResponse = (status: number, body: unknown) => + new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }) + +test('request sends a JSON body with the content type set once', async () => { + const fetchMock = vi.fn(async (_input: RequestInfo | URL, _init?: RequestInit) => + jsonHttpResponse(200, { ok: true }), + ) + vi.stubGlobal('fetch', fetchMock) + + await apiClient.post('/things', { body: { name: 'x' } }) + + const init = fetchMock.mock.calls[0]?.[1] as RequestInit + const headers = init.headers as Headers + expect(headers.get('Content-Type')).toBe('application/json') + expect(init.body).toBe(JSON.stringify({ name: 'x' })) +}) + +test('request without a body sets no content type and passes no body', async () => { + const fetchMock = vi.fn(async (_input: RequestInfo | URL, _init?: RequestInit) => + jsonHttpResponse(200, []), + ) + vi.stubGlobal('fetch', fetchMock) + + await apiClient.get('/things') + + const init = fetchMock.mock.calls[0]?.[1] as RequestInit + expect((init.headers as Headers).get('Content-Type')).toBeNull() + expect(init.body).toBeUndefined() +}) + +test('a 204 response parses to null; a text response parses as text', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(null, { status: 204 })), + ) + expect(await apiClient.get('/things')).toBeNull() + + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('ok-text', { status: 200 })), + ) + expect(await apiClient.get('/things')).toBe('ok-text') +}) + +test('an error response throws the mapped ApiError', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => jsonHttpResponse(403, { detail: 'FORBIDDEN' })), + ) + + await expect(apiClient.get('/things')).rejects.toMatchObject({ code: 'FORBIDDEN', status: 403 }) +}) diff --git a/apps/demo/src/shared/api/errors.test.ts b/apps/demo/src/shared/api/errors.test.ts new file mode 100644 index 00000000..6016bc73 --- /dev/null +++ b/apps/demo/src/shared/api/errors.test.ts @@ -0,0 +1,50 @@ +import { expect, test } from 'vite-plus/test' + +import { ApiAuthError, ApiError, ApiValidationError, createApiError, isApiError } from './errors' + +const response = (status: number) => new Response(null, { status }) + +test('401 payloads become ApiAuthError', () => { + const error = createApiError(response(401), { detail: 'Token expired' }) + expect(error).toBeInstanceOf(ApiAuthError) + expect(error.message).toBe('Authentication is required') + expect(isApiError(error)).toBe(true) +}) + +test('422 payloads become ApiValidationError carrying extracted issues', () => { + const error = createApiError(response(422), { + detail: [{ loc: ['body', 'email'], msg: 'Invalid address' }], + }) + if (!(error instanceof ApiValidationError)) throw new Error('expected ApiValidationError') + expect(error.issues).toEqual([{ loc: ['body', 'email'], msg: 'Invalid address' }]) +}) + +// Some backends signal validation with a 400 carrying an issues array instead +// of a 422 — the array shape alone selects ApiValidationError. +test('a non-422 payload with an error array is still a validation error', () => { + const error = createApiError(response(400), { + error: [{ loc: ['password'], msg: 'Too short' }], + }) + if (!(error instanceof ApiValidationError)) throw new Error('expected ApiValidationError') + expect(error.issues).toHaveLength(1) +}) + +test('a plain error object becomes the message; a string becomes the code', () => { + expect( + createApiError(response(400), { error: { message: 'Invalid email or password' } }).message, + ).toBe('Invalid email or password') + expect(createApiError(response(403), { detail: 'FORBIDDEN' }).code).toBe('FORBIDDEN') + expect(createApiError(response(500), null).message).toBe('API request failed with status 500') +}) + +test('validation extraction drops entries that are not issues', () => { + const error = new ApiValidationError(422, { + detail: [{ loc: ['email'], msg: 'Required' }, 'not-an-issue', { msg: 'no loc' }, 42], + }) + expect(error.issues).toEqual([{ loc: ['email'], msg: 'Required' }]) +}) + +test('a scalar error payload carries no code', () => { + expect(new ApiError(500, 'boom').code).toBeNull() + expect(new ApiError(500, ['unexpected array']).code).toBeNull() +}) diff --git a/apps/demo/src/shared/api/errors.ts b/apps/demo/src/shared/api/errors.ts new file mode 100644 index 00000000..7f455b77 --- /dev/null +++ b/apps/demo/src/shared/api/errors.ts @@ -0,0 +1,110 @@ +type ErrorDetail = string | Record | Array +type ErrorPayload = + | { detail: ErrorDetail } + | { error: ErrorDetail | Array | { message?: string } } + | Record + +export type ApiValidationIssue = { + type?: string + loc: Array + msg: string + input?: unknown +} + +export class ApiError extends Error { + readonly status: number + readonly payload: unknown + readonly code: string | null + + constructor(status: number, payload: unknown, message?: string) { + const code = extractErrorCode(payload) + super(message ?? code ?? `API request failed with status ${status}`) + this.name = 'ApiError' + this.status = status + this.payload = payload + this.code = code + } +} + +export class ApiAuthError extends ApiError { + constructor(payload: unknown) { + super(401, payload, 'Authentication is required') + this.name = 'ApiAuthError' + } +} + +export class ApiValidationError extends ApiError { + readonly issues: Array + + constructor(status: number, payload: unknown) { + super(status, payload, 'Request validation failed') + this.name = 'ApiValidationError' + this.issues = extractValidationIssues(payload) + } +} + +export function isApiError(error: unknown): error is ApiError { + return error instanceof ApiError +} + +export function isApiValidationError(error: unknown): error is ApiValidationError { + return error instanceof ApiValidationError +} + +function isValidationIssue(value: unknown): value is ApiValidationIssue { + if (!value || typeof value !== 'object') return false + const issue = value as Partial + return Array.isArray(issue.loc) && typeof issue.msg === 'string' +} + +function extractValidationIssues(payload: unknown): Array { + if (!payload || typeof payload !== 'object') return [] + const data = payload as ErrorPayload + const source = 'error' in data ? data.error : 'detail' in data ? data.detail : [] + return Array.isArray(source) ? source.filter(isValidationIssue) : [] +} + +// The extractor is split so no helper's cyclomatic complexity tops the +// fallow CRAP threshold (in CI, CRAP assumes zero coverage: comp^2 + comp). +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null + +const validationCode = (items: unknown[]): string | null => + items.length > 0 ? 'VALIDATION_ERROR' : null + +const stringField = (record: Record, key: string): string | null => + typeof record[key] === 'string' ? (record[key] as string) : null + +const codeFromDetail = (detail: unknown): string | null => { + if (typeof detail === 'string') return detail + if (Array.isArray(detail)) return validationCode(detail) + if (!isRecord(detail)) return null + return stringField(detail, 'code') +} + +const codeFromError = (error: unknown): string | null => { + if (typeof error === 'string') return error + if (Array.isArray(error)) return validationCode(error) + if (!isRecord(error)) return null + return stringField(error, 'message') +} + +function extractErrorCode(payload: unknown): string | null { + if (!isRecord(payload)) return null + if ('detail' in payload) { + const code = codeFromDetail(payload['detail']) + if (code !== null) return code + } + return 'error' in payload ? codeFromError(payload['error']) : null +} + +export function createApiError(response: Response, payload: unknown) { + if (response.status === 401) return new ApiAuthError(payload) + if ( + response.status === 422 || + (payload && typeof payload === 'object' && 'error' in payload && Array.isArray(payload.error)) + ) { + return new ApiValidationError(response.status, payload) + } + return new ApiError(response.status, payload) +} diff --git a/apps/demo/src/shared/api/index.ts b/apps/demo/src/shared/api/index.ts index 0e5c84a1..3d561cb1 100644 --- a/apps/demo/src/shared/api/index.ts +++ b/apps/demo/src/shared/api/index.ts @@ -1,5 +1,22 @@ import { abortVar } from '@reatom/core' +import { createApiError } from './errors' + +export { + // fallow-ignore-next-line unused-export + ApiAuthError, + // fallow-ignore-next-line unused-export + ApiError, + // fallow-ignore-next-line unused-export + ApiValidationError, + // fallow-ignore-next-line unused-export + createApiError, + isApiError, + isApiValidationError, + type ApiValidationIssue, +} from './errors' +export { applyApiValidationToFields } from './validation' + const API_PREFIX = '/api' export function composeApiUrl(path = '') { @@ -10,22 +27,6 @@ export function composeApiUrl(path = '') { return `${API_PREFIX}${normalized}` } -class ApiError extends Error { - readonly status: number - readonly payload: unknown - - constructor(status: number, payload: unknown) { - super(`API request failed with status ${status}`) - this.name = 'ApiError' - this.status = status - this.payload = payload - } -} - -export function isApiError(error: unknown): error is ApiError { - return error instanceof ApiError -} - type RequestOptions = Omit & { body?: unknown } @@ -71,7 +72,7 @@ async function request(path: string, options: RequestOptions = {}) { const payload = await parseResponsePayload(response) if (!response.ok) { - throw new ApiError(response.status, payload) + throw createApiError(response, payload) } return payload as TResponse diff --git a/apps/demo/src/shared/api/validation.test.ts b/apps/demo/src/shared/api/validation.test.ts new file mode 100644 index 00000000..3c1d5ce3 --- /dev/null +++ b/apps/demo/src/shared/api/validation.test.ts @@ -0,0 +1,86 @@ +import { reatomField, reatomForm } from '@reatom/core' +import { expect, test } from 'vite-plus/test' + +import { ApiValidationError } from './errors' +import { applyApiValidationToFields } from './validation' + +const validationError = (issues: Array<{ loc: Array; msg: string }>) => + new ApiValidationError(422, { detail: issues }) + +const formWithFields = () => + reatomForm({ + email: reatomField('', { name: 'test.serverFields.email' }), + password: reatomField('', { name: 'test.serverFields.password' }), + }) + +test('a server issue lands under its field as a server-sourced error', () => { + const form = formWithFields() + const unmapped = applyApiValidationToFields( + validationError([{ loc: ['body', 'email'], msg: 'Already registered' }]), + form.fields, + ) + + expect(unmapped).toEqual([]) + expect(form.fields.email.validation.errors().at(-1)).toEqual({ + source: 'server', + message: 'Already registered', + }) +}) + +test('issue locs match by field-name suffix, so envelope prefixes still map', () => { + const form = formWithFields() + // `body.email` and `user.credentials.password` both end in a field name. + const unmapped = applyApiValidationToFields( + validationError([ + { loc: ['body', 'email'], msg: 'a' }, + { loc: ['user', 'credentials', 'password'], msg: 'b' }, + ]), + form.fields, + ) + expect(unmapped).toEqual([]) + expect(form.fields.email.validation.errors()).toHaveLength(1) + expect(form.fields.password.validation.errors()).toHaveLength(1) +}) + +test('issues no field claims are returned unmapped instead of dropped', () => { + const form = formWithFields() + const unmapped = applyApiValidationToFields( + validationError([{ loc: ['billing', 'vat_id'], msg: 'Invalid VAT ID' }]), + form.fields, + ) + expect(unmapped).toHaveLength(1) + expect(unmapped[0]?.msg).toBe('Invalid VAT ID') +}) + +test('re-mapping clears the previous server errors first, so they never pile up', () => { + const form = formWithFields() + applyApiValidationToFields(validationError([{ loc: ['email'], msg: 'first' }]), form.fields) + applyApiValidationToFields(validationError([{ loc: ['email'], msg: 'second' }]), form.fields) + expect(form.fields.email.validation.errors()).toHaveLength(1) + expect(form.fields.email.validation.errors().at(-1)?.message).toBe('second') +}) + +test('non-validation errors are ignored, leaving field state untouched', () => { + const form = formWithFields() + const before = form.fields.email.validation.errors().length + expect(applyApiValidationToFields(new Error('network down'), form.fields)).toEqual([]) + expect(form.fields.email.validation.errors().length).toBe(before) +}) + +// The point of the keepErrorOnChange merge: a server error is the only error a +// field cannot re-check by editing, so without the next-keystroke drop it +// would block every later submit locally. Editing must clear it and hand the +// verdict back to the server. +test('a mapped server error is dropped by the next edit to the field', async () => { + const form = formWithFields() + applyApiValidationToFields( + validationError([{ loc: ['email'], msg: 'Already registered' }]), + form.fields, + ) + expect(form.fields.email.validation.errors().length).toBe(1) + + form.fields.email.change('someone-else@example.com') + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(form.fields.email.validation.errors().length).toBe(0) +}) diff --git a/apps/demo/src/shared/api/validation.ts b/apps/demo/src/shared/api/validation.ts new file mode 100644 index 00000000..b03ea64c --- /dev/null +++ b/apps/demo/src/shared/api/validation.ts @@ -0,0 +1,61 @@ +import type { Action, ArrayAtom } from '@reatom/core' + +import { type ApiValidationIssue, isApiValidationError } from './errors' + +type FieldWithValidation = { + options: { merge: (patch: { keepErrorOnChange: boolean }) => unknown } + validation: { + clearErrors: Action<[...sources: Array], unknown> + errors: ArrayAtom<{ source: string; message: string }> + } +} + +type FieldRecord = Record + +const SERVER_VALIDATION_SOURCE = 'server' + +export function applyApiValidationToFields( + error: unknown, + fields: FieldRecord, +): Array { + if (!isApiValidationError(error)) return [] + + for (const field of Object.values(fields)) { + field.validation.clearErrors(SERVER_VALIDATION_SOURCE) + } + + const unmapped: Array = [] + for (const issue of error.issues) { + const fieldName = findIssueFieldName(issue, fields) + if (!fieldName) { + unmapped.push(issue) + continue + } + const field = fields[fieldName] + if (!field) continue + field.validation.errors.push({ + source: SERVER_VALIDATION_SOURCE, + message: issue.msg, + }) + // A server error is the only kind a field cannot re-check on its own, so + // unless editing clears it the field stays invalid forever — and the form + // refuses every later submit locally, without ever asking the server again. + // `keepErrorOnChange: false` makes the next keystroke drop it; the server + // gets to judge the new value on the next submit. + field.options.merge({ keepErrorOnChange: false }) + } + return unmapped +} + +function findIssueFieldName(issue: ApiValidationIssue, fields: FieldRecord) { + const loc = issue.loc.map(String) + for (let index = 0; index < loc.length; index++) { + const suffix = loc.slice(index).join('.') + if (suffix in fields) return suffix + } + for (let index = loc.length - 1; index >= 0; index--) { + const segment = loc[index] + if (segment && segment in fields) return segment + } + return null +} diff --git a/apps/demo/src/shared/mocks/utils.ts b/apps/demo/src/shared/mocks/utils.ts index 4eca5b43..799542ba 100644 --- a/apps/demo/src/shared/mocks/utils.ts +++ b/apps/demo/src/shared/mocks/utils.ts @@ -27,6 +27,13 @@ export const to500 = (message = 'Internal Server Error') => { throw new Error500(message) } +// Server-side field validation, FastAPI style. `loc` may carry envelope +// prefixes (e.g. ['body', 'email']) — the client maps issues to form fields +// by field-name suffix. +export const to422 = (issues: Array<{ loc: Array; msg: string }>) => { + throw HttpResponse.json({ detail: issues }, { status: 422 }) +} + export function withRetrySuccess( resolver: TResolver, failures = 2, diff --git a/apps/demo/src/shared/model/index.ts b/apps/demo/src/shared/model/index.ts index 0b1b0fe7..05844dcb 100644 --- a/apps/demo/src/shared/model/index.ts +++ b/apps/demo/src/shared/model/index.ts @@ -8,6 +8,12 @@ export { withMatchMobileHeaderOverride, } from './headerTrail' export type { HeaderTrailDescriptor } from './headerTrail' +export { + // fallow-ignore-next-line unused-export + readPersistRecord, + // fallow-ignore-next-line unused-export + withAppWebStorage, +} from './persist' export { documentTitleAtom } from './documentTitle' export { localeAtom, reatomLoc } from './locale' export { themePreferenceAtom } from './theme' diff --git a/apps/demo/src/shared/model/persist.test.ts b/apps/demo/src/shared/model/persist.test.ts new file mode 100644 index 00000000..836034b0 --- /dev/null +++ b/apps/demo/src/shared/model/persist.test.ts @@ -0,0 +1,124 @@ +import { atom } from '@reatom/core' +import { afterEach, expect, test, vi } from 'vite-plus/test' + +import { readPersistRecord, withAppWebStorage } from './persist' + +class MemoryStorage implements Storage { + readonly store = new Map() + + get length() { + return this.store.size + } + + clear() { + this.store.clear() + } + + getItem(key: string) { + return this.store.get(key) ?? null + } + + key(index: number) { + return [...this.store.keys()][index] ?? null + } + + removeItem(key: string) { + this.store.delete(key) + } + + setItem(key: string, value: string) { + this.store.set(key, value) + } +} + +afterEach(() => { + vi.unstubAllGlobals() +}) + +// withAppWebStorage reads globalThis.localStorage when the adapter factory +// runs, so a stub installed before model modules are imported is honored. +// withLocalStorage cannot do this: it captures storage once when @reatom/core +// loads, before any test stub can be installed. +test('a localStorage stub installed before the factory call is honored', () => { + const localStorage = new MemoryStorage() + vi.stubGlobal('localStorage', localStorage) + + const adapter = withAppWebStorage('test.persist') + const persisted = atom('a', 'test.persistedAtom').extend(adapter({ key: 'test.persist' })) + persisted.set('b') + + expect(localStorage.getItem('test.persist')).toContain('"data":"b"') +}) + +test('a later atom with the same key restores the persisted state', () => { + const localStorage = new MemoryStorage() + vi.stubGlobal('localStorage', localStorage) + + const adapter = withAppWebStorage('test.persist') + const first = atom('a', 'test.roundtripFirstAtom').extend(adapter({ key: 'test.roundtrip' })) + first.set('b') + + const second = atom('a', 'test.roundtripSecondAtom').extend(adapter({ key: 'test.roundtrip' })) + + expect(second()).toBe('b') +}) + +test('absent localStorage falls back to memory and the atom still works', () => { + vi.stubGlobal('localStorage', undefined) + + const adapter = withAppWebStorage('test.memory') + const persisted = atom('a', 'test.memoryAtom').extend(adapter({ key: 'test.memory' })) + persisted.set('b') + + expect(persisted()).toBe('b') + expect(readPersistRecord('test.memory')).toBeUndefined() +}) + +test('a localStorage getter that throws falls back to memory', () => { + // Opaque origins and blocked site data make the getter itself throw. + vi.stubGlobal('localStorage', undefined) + Object.defineProperty(globalThis, 'localStorage', { + configurable: true, + get() { + throw new DOMException('Access denied', 'SecurityError') + }, + }) + + const adapter = withAppWebStorage('test.blocked') + const persisted = atom('a', 'test.blockedAtom').extend(adapter({ key: 'test.blocked' })) + persisted.set('b') + + expect(persisted()).toBe('b') + expect(readPersistRecord('test.blocked')).toBeUndefined() +}) + +test('readPersistRecord returns the payload of a live record', () => { + const localStorage = new MemoryStorage() + vi.stubGlobal('localStorage', localStorage) + localStorage.setItem( + 'test.record', + JSON.stringify({ + data: 'payload', + id: 'record-1', + timestamp: Date.now(), + to: Date.now() + 60_000, + version: 0, + }), + ) + + expect(readPersistRecord('test.record')).toBe('payload') +}) + +test('readPersistRecord rejects expired, malformed, and missing records', () => { + const localStorage = new MemoryStorage() + vi.stubGlobal('localStorage', localStorage) + + localStorage.setItem('test.expired', JSON.stringify({ data: 'old', to: Date.now() - 1 })) + localStorage.setItem('test.malformed', 'not json') + localStorage.setItem('test.notRecord', JSON.stringify({ data: 'no expiry' })) + + expect(readPersistRecord('test.expired')).toBeUndefined() + expect(readPersistRecord('test.malformed')).toBeUndefined() + expect(readPersistRecord('test.notRecord')).toBeUndefined() + expect(readPersistRecord('test.missing')).toBeUndefined() +}) diff --git a/apps/demo/src/shared/model/persist.ts b/apps/demo/src/shared/model/persist.ts new file mode 100644 index 00000000..cb13bdd5 --- /dev/null +++ b/apps/demo/src/shared/model/persist.ts @@ -0,0 +1,47 @@ +import { + createMemStorage, + isPersistRecord, + reatomPersist, + reatomPersistWebStorage, +} from '@reatom/core' + +const memoryFallback = createMemStorage({ name: 'appPersist' }) + +/** + * The `localStorage` getter throws `SecurityError` on opaque origins and when + * the browser blocks site data, so every read goes through this. + */ +function webStorage(): Storage | undefined { + try { + return globalThis.localStorage ?? undefined + } catch { + return undefined + } +} + +/** + * Web-storage persist adapter built when the caller's module loads, so a + * localStorage stub installed before that import is honored — the persistence + * tests depend on it. `withLocalStorage` cannot do this: it captures storage + * once when `@reatom/core` loads. Falls back to memory where localStorage is + * absent or blocked. + */ +export const withAppWebStorage = (name: string) => { + const storage = webStorage() + return storage ? reatomPersistWebStorage(name, storage) : reatomPersist(memoryFallback) +} + +/** Read a Reatom web-storage record without coupling callers to its shape. */ +export function readPersistRecord(key: string): T | undefined { + const storage = webStorage() + if (!storage) return undefined + try { + const raw = storage.getItem(key) + if (!raw) return undefined + const value: unknown = JSON.parse(raw) + if (!isPersistRecord(value) || value.to < Date.now()) return undefined + return value.data as T + } catch { + return undefined + } +} diff --git a/apps/demo/src/shared/reatom/forms.test.ts b/apps/demo/src/shared/reatom/forms.test.ts new file mode 100644 index 00000000..0ff6f52b --- /dev/null +++ b/apps/demo/src/shared/reatom/forms.test.ts @@ -0,0 +1,261 @@ +import { clearStack, context, reatomField, reatomForm, STACK, wrap } from '@reatom/core' +import { afterEach, expect, test } from 'vite-plus/test' + +import { + formAlertMessage, + visibleFieldError, + withFormAutoFocusOnError, + withFormSubmitHandler, + withSavedState, +} from './forms' + +afterEach(() => context.reset()) + +// `submit.onFulfill` hooks run in the next cleanup-queue tick, not synchronously +// with the awaited submit promise, so every assertion about post-save state has +// to let the queue flush first. +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)) + +type TestValues = { title: string } + +// The template ships no schema validator, so the schema-path tests drive the +// Standard Schema contract directly — reatomForm only calls +// `schema['~standard'].validate(state)` and maps the returned issues by path, +// so a minimal structural implementation stands in for a validator library. +// Typed structurally against the reatomForm `schema` option rather than +// @standard-schema/spec, which is only a transitive (non-importable) dependency. +type TestSchema = { + '~standard': { + version: 1 + vendor: 'test' + validate: ( + value: unknown, + ) => { value: State } | { issues: ReadonlyArray<{ message: string; path?: string[] }> } + } +} + +const requiredFieldsSchema = >( + fields: ReadonlyArray<[keyof State, string]>, +): TestSchema => ({ + '~standard': { + version: 1, + vendor: 'test', + validate: (value) => { + const state = value as State + const issues = fields.flatMap(([key, message]) => + (state[key] ?? '').trim() ? [] : [{ message, path: [String(key)] }], + ) + return issues.length > 0 ? { issues } : { value: state } + }, + }, +}) + +const reatomTestForm = ( + onSubmit: (values: TestValues) => Promise, + options?: Parameters[0], +) => reatomForm({ title: 'saved' }, { name: 'test.form', onSubmit }).extend(withSavedState(options)) + +test('a saved form rebaselines on the submitted values and reports itself clean', async () => { + const form = reatomTestForm(async (values) => values) + + form.fields.title.change('edited') + expect(form.focus().dirty).toBe(true) + + await form.submit() + await flush() + + expect(form.fields.title.value()).toBe('edited') + expect(form.focus().dirty).toBe(false) +}) + +// The failure this guards against: rebaselining on the form's LIVE state would +// adopt whatever was typed while the request was in flight as "saved", so an +// edit that never reached the server would read as clean — and the disclosure +// card would collapse over it, showing a summary row with the old value. +test('an edit typed while the save is in flight stays unsaved', async () => { + let release: (() => void) | undefined + const inFlight = new Promise((resolve) => { + release = resolve + }) + const form = reatomTestForm(async (values) => { + await wrap(inFlight) + return values + }) + + form.fields.title.change('sent') + const submitted = form.submit() + // Typed after the request left, so it was never part of the payload. + form.fields.title.change('typed during save') + release?.() + await submitted + await flush() + + expect(form.fields.title.value()).toBe('typed during save') + expect(form.focus().dirty).toBe(true) +}) + +test('onSaved runs once the save resolves, and not on a failed one', async () => { + let saved = 0 + const form = reatomTestForm(async (values) => values, { onSaved: () => (saved += 1) }) + form.fields.title.change('ok') + await form.submit() + await flush() + expect(saved).toBe(1) + + const failing = reatomTestForm( + async () => { + throw new Error('nope') + }, + { onSaved: () => (saved += 1) }, + ) + failing.fields.title.change('boom') + await failing.submit().catch(() => {}) + await flush() + expect(saved).toBe(1) +}) + +test('returning nothing clears a write-only form back to its initial baseline', async () => { + // Write-only forms start empty (nothing to read back) and return null so + // `withSavedState` resets them rather than rebaselining on the secret. + const form = reatomForm( + { secret: '' }, + { + name: 'test.writeOnly', + onSubmit: async () => null, + }, + ).extend(withSavedState()) + form.fields.secret.change('typed-secret') + await form.submit() + await flush() + + expect(form.fields.secret.value()).toBe('') + expect(form.focus().dirty).toBe(false) +}) + +test('form alert keeps a failure visible when no field owns it', async () => { + const form = reatomTestForm(async () => { + throw new Error('Request failed') + }) + + await form.submit().catch(() => {}) + expect(form.validation().errors).toHaveLength(0) + expect(formAlertMessage(form)).toBe('Request failed') +}) + +// A partially mapped server error leaves field errors on screen and a +// remainder no field shows. The caller's predicate decides, not the field count. +test('form alert shows an unhandled failure even while a field shows an error', async () => { + const form = reatomForm( + { title: reatomField('', { name: 'test.partial.title' }) }, + { + name: 'test.partial', + onSubmit: async () => { + form.fields.title.validation.errors.push({ source: 'server', message: 'Taken' }) + throw new Error('Request failed') + }, + }, + ) + + await form.submit().catch(() => {}) + expect(form.validation().errors).toHaveLength(1) + expect(formAlertMessage(form)).toBeNull() + expect(formAlertMessage(form, () => false)).toBe('Request failed') + expect(formAlertMessage(form, () => true)).toBeNull() +}) + +test('form alert hides a retained failure while a re-submission is pending', async () => { + let attempts = 0 + const gates: Array<() => void> = [] + const form = reatomTestForm(async (values) => { + attempts += 1 + if (attempts === 1) throw new Error('Request failed') + await new Promise((resolve) => gates.push(resolve)) + return values + }) + + await form.submit().catch(() => {}) + expect(formAlertMessage(form)).toBe('Request failed') + + const retry = form.submit() + // The failed attempt's error is retained until the retry fulfills, but the + // pending attempt owns the user's attention now. + expect(form.submit.ready()).toBe(false) + expect(formAlertMessage(form)).toBeNull() + await flush() + gates[0]?.() + await retry + expect(formAlertMessage(form)).toBeNull() +}) + +test('a form schema blocks onSubmit and owns the visible field error', async () => { + let submits = 0 + const form = reatomForm( + { title: reatomField('', { name: 'test.validated.title' }) }, + { + name: 'test.validated', + validateOnBlur: true, + keepErrorOnChange: false, + schema: requiredFieldsSchema([['title', 'Required']]), + onSubmit: (values) => { + submits += 1 + return values + }, + }, + ).extend(withFormSubmitHandler()) + + await form.submit().catch(() => {}) + expect(submits).toBe(0) + expect(visibleFieldError(form.fields.title)).toBe('Required') + expect(formAlertMessage(form)).toBeNull() + + form.fields.title.set('fixed') + expect(visibleFieldError(form.fields.title)).toBeUndefined() + expect(formAlertMessage(form)).toBeNull() + form.fields.title.focus.in() + form.fields.title.focus.out() + await flush() + + await form.submit() + expect(submits).toBe(1) +}) + +test('submit handler prevents native submission and delegates to form validation', async () => { + let prevented = 0 + let submits = 0 + const form = reatomForm( + { title: 'ready' }, + { name: 'test.submitHandler', onSubmit: () => (submits += 1) }, + ).extend(withFormSubmitHandler()) + const hostHandler = wrap(form.handleSubmit) + const testStack = [...STACK] + clearStack() + try { + hostHandler({ preventDefault: () => (prevented += 1) }) + } finally { + STACK.push(...testStack) + } + await flush() + + expect(prevented).toBe(1) + expect(submits).toBe(1) +}) + +test('auto focus targets the first invalid field after rejected submit', async () => { + let focused = 0 + const form = reatomForm( + { first: '', second: '' }, + { + name: 'test.autoFocus', + schema: requiredFieldsSchema<{ first: string; second: string }>([ + ['first', 'First is required'], + ['second', 'Second is required'], + ]), + }, + ).extend(withFormAutoFocusOnError()) + form.fields.first.elementRef.set({ focus: () => (focused += 1) }) + + await form.submit().catch(() => {}) + await flush() + + expect(focused).toBe(1) +}) diff --git a/apps/demo/src/shared/reatom/forms.ts b/apps/demo/src/shared/reatom/forms.ts new file mode 100644 index 00000000..9e43ea0f --- /dev/null +++ b/apps/demo/src/shared/reatom/forms.ts @@ -0,0 +1,216 @@ +import type { + Action, + AtomLike, + Computed, + FieldAtom, + FieldFocus, + Form, + SubmitAction, +} from '@reatom/core' + +import { action, atom, withCallHook } from '@reatom/core' + +type PreventableEvent = { preventDefault(): void } + +/** + * Adds a host-event submit action so ordinary forms do not repeat DOM bridging boilerplate. + * Adapted from reatom/reusables `with-form-submit-handler`. + * The component must still pass `wrap(form.handleSubmit)` to its host-managed form element. + */ +export const withFormSubmitHandler = + () => + >(form: Target) => { + // `submit` runs `validation.trigger` before `onSubmit`, so its outcome tells + // a local validation failure apart from a server rejection. Counting field + // errors cannot: a 422 mapped onto fields leaves errors there too. + const submitValidationError = atom(false, `${form.name}.submitValidationError`) + form.validation.trigger.onReject.extend(withCallHook(() => submitValidationError.set(true))) + form.validation.trigger.onFulfill.extend(withCallHook(() => submitValidationError.set(false))) + + return { + handleSubmit: action((event?: PreventableEvent) => { + event?.preventDefault() + form.submit() + }, `${form.name}.handleSubmit`), + submitValidationError, + } + } + +/** + * Focuses the first invalid field after submit rejection when its element ref is connected. + * Adapted from reatom/reusables `with-form-auto-focus-on-error`. + */ +export const withFormAutoFocusOnError = + () => + >(form: Target): Target => { + form.submit.onReject.extend( + withCallHook(() => { + form + .fieldsList() + .find((field) => field.validation().error) + ?.elementRef() + ?.focus() + }), + ) + return form + } + +/** + * Scrolls the first invalid field or section-level error into view when + * submit is rejected. + * + * `withFormAutoFocusOnError` only reaches fields with a wired `elementRef`. + * Fields that validate a whole array of row objects (entry lists, item + * mappings, …) have no single input to point an `elementRef` at, so their + * error renders below the row list instead as a plain `role="alert"` element + * — and every Ark UI `Field.Root` already stamps a `data-invalid` attribute + * on itself (and its descendants) when invalid, boolean-style — present with + * an empty value, not `data-invalid="true"` — so `[data-invalid]` (not + * `[data-invalid="true"]`) is what actually matches. With no wiring required, + * this scrolls to whichever of the two the submit rejection left on screen — + * the first invalid thing in document order — so a multi-section wizard form + * no longer strands the user next to the submit button while the actual + * unfilled field sits off-screen above the fold. Runs after a frame so the + * rejection's error text has painted first. + */ +// fallow-ignore-next-line unused-export +export const withFormScrollToErrorOnReject = + () => + >(form: Target): Target => { + form.submit.onReject.extend( + withCallHook(() => { + requestAnimationFrame(() => { + const target = document.querySelector('[data-invalid], [role="alert"]') + target?.scrollIntoView({ behavior: 'smooth', block: 'center' }) + target + ?.querySelector('input, select, button, textarea') + ?.focus({ preventScroll: true }) + }) + }), + ) + return form + } + +/** The structural slice of a `reatomForm` this extension drives. */ +type FormLike = AtomLike & { + focus: Computed + submit: SubmitAction + init: Action + reset: Action +} + +/** The structural slice `formAlertMessage` reads. */ +type FormWithSubmitError = { + validation: () => { errors: ReadonlyArray } + submit: { + error: () => Error | null | undefined + ready?: () => boolean + } + submitValidationError?: () => boolean +} + +/** + * The message a form-level alert may show, or `null` for no alert. + * + * The alert exists for failures no field can carry — a network error, a server + * message about the record as a whole. Two kinds of failure reach `submit.error` + * that are already on the fields, and repeating them there prints the same + * sentence twice: + * + * A field validation failure reaches `submit.error` as a plain Error carrying a + * copy of the first field message. API validation can do the same after its + * issues are mapped onto fields. In either case, the aggregate validation state + * tells us the message already has a field-level owner. + * + * A mapped API error outlives the field error it produced: editing the field + * clears its validation entry, but `submit.error()` keeps the rejected request + * until a submit succeeds. Callers that map a submit error elsewhere pass an + * `isHandled` predicate so that stale request error cannot migrate into this + * alert. When the predicate is passed, its verdict is final: a partial mapping + * leaves field errors on screen AND a remainder only the alert can show, so + * the aggregate field-error check would hide that remainder. + * + * While a submission is pending the alert is suppressed: the retained error + * belongs to the previous attempt, and the loading state is the feedback. + */ +export function formAlertMessage(form: FormWithSubmitError, isHandled?: (error: Error) => boolean) { + const error = form.submit.error() + // `submit.error()` is retained until a later submit fulfills, so while a + // re-submission is in flight the stale text would sit beside the loading + // state. Nothing is announced until that attempt settles. + if (!error || form.submit.ready?.() === false) return null + if (form.submitValidationError?.()) return null + if (isHandled) return isHandled(error) ? null : error.message + return form.validation().errors.length > 0 ? null : error.message +} + +/** + * The field error that should currently be visible to the user. + * + * `keepErrorOnChange: false` keeps the last issue in Reatom for bookkeeping but + * drops its `triggered` flag. Reading only `validation.error` would therefore + * leave stale copy visible while the user fixes the value. + */ +export function visibleFieldError(field: Pick, 'validation'>) { + const validation = field.validation() + return validation.triggered ? validation.error : undefined +} + +/** + * What a settings form must do once its save lands, so the UI stops claiming + * there is something left to save. The post-save state is driven ENTIRELY by + * what `onSubmit` RETURNS: + * + * - a truthy payload makes those values the form's new baseline, so it reads as + * clean: the submit CTA demotes, the reset affordance disappears, and the + * inputs keep showing what was saved. Applied with `init`, not `reset`. + * - returning nothing (or `null`) clears the form to its empty baseline with + * `reset` — for write-only credential forms, where the secret must not + * linger and there is nothing to read back. + * + * `onSaved` runs after that. Disclosure cards use it to collapse back to their + * summary rows — landing on rows that now state the new values is what confirms + * the save, without a toast claiming it. + * + * Two details matter about the rebaseline path: + * + * - the returned payload is what was actually persisted. Reading the form's + * live state at fulfilment instead would adopt anything typed while the + * request was in flight — edits that were never sent — as "saved". (Snapshot + * at call time does not work either: `withCallHook` on `submit` fires after + * the async body, by which point the state has already moved.) + * - `init` rebaselines without overwriting the current values, so an in-flight + * edit survives AND keeps the form dirty. `reset(values)` would clobber it — + * verified against @reatom/core. + * + * So a form with persistent values must `return` them from `onSubmit`; a + * write-only form returns nothing and is cleared by `reset`. Do not use the + * form's own `resetOnSubmit` option — this extension is the single place + * post-save clearing is decided, and two owners of that decision disagree. + * + * Applied to the form itself (not to a component) because both effects are + * state, not presentation: any card, drawer or page rendering the form gets + * the same post-save behaviour. + * + * Note: `reatomForm` clears `submit.data`/`submit.error` on reset, so never + * build a "saved" indicator on `submit.data()`. + */ +export const withSavedState = + ({ onSaved }: { onSaved?: () => void } = {}) => + (target: Target) => { + // `onFulfill` fires only on resolution — withAsync routes aborts to + // `onSettle` and failures to `onReject` — and its hook body already runs + // inside a Reatom frame, so no `wrap` is needed here. Its first param is + // whatever `onSubmit` returned. + target.submit.onFulfill.extend( + withCallHook((_call, [persisted]) => { + if (persisted) { + target.init(persisted) + } else { + target.reset() + } + onSaved?.() + }), + ) + return target + } diff --git a/apps/demo/src/shared/reatom/index.ts b/apps/demo/src/shared/reatom/index.ts index 8475d35a..ef61d6b6 100644 --- a/apps/demo/src/shared/reatom/index.ts +++ b/apps/demo/src/shared/reatom/index.ts @@ -1 +1,10 @@ +export { + formAlertMessage, + visibleFieldError, + withFormSubmitHandler, + withFormAutoFocusOnError, + // fallow-ignore-next-line unused-export + withFormScrollToErrorOnReject, + withSavedState, +} from './forms' export { withResizeObserver } from './resize-observer' diff --git a/docs/patterns.md b/docs/patterns.md new file mode 100644 index 00000000..0fd37672 --- /dev/null +++ b/docs/patterns.md @@ -0,0 +1,63 @@ +# Patterns + +This doc follows the source-first approach in `docs/README.md`. + +## Overview + +The template ships mechanisms taken from production apps, each with the +decision that makes it correct. Every pattern has a working demo in +`apps/demo`. The long-form write-ups live on the site +(`site/src/content/patterns/`). This doc is the short index, and the comments +at the decision points in the source are the ground truth. + +For general Reatom conventions, see `docs/reatom-patterns.md`. For +extension-point mechanics, see `docs/reatom-extensions.md`. + +## Read Source First + +| Pattern | The decision | Template (shipped) | Demo (proves it) | +| ------------------------- | --------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------- | +| Form save semantics | What `onSubmit` returns owns the post-save state: a returned payload becomes the baseline, nothing resets the form. | `packages/create-karkas/template/src/shared/reatom/forms.ts` | `apps/demo/src/pages/settings/model/settingsForm.ts`, `…/articles/model/articleDetailModel.ts` | +| Form-level alert gating | The alert shows only failures no field owns, and a mapped request error never reappears in it. | `formAlertMessage` in `…/shared/reatom/forms.ts` | `apps/demo/src/pages/login/ui/LoginPage.tsx` | +| Visible field errors | The error reads `triggered` as well as `error`, so the old message clears while the user fixes the value. | `visibleFieldError` in `…/shared/reatom/forms.ts` | `apps/demo/src/pages/login/ui/LoginPage.tsx` | +| Server validation mapping | A field cannot re-check a server error, so 422 issues map onto fields by name suffix and clear on the next edit. | `src/shared/api/validation.ts`, `errors.ts` | `apps/demo/src/pages/login/model/routes.tsx` | +| Server states | Each entity exports named MSW scenarios (`loading`, `error`, `retrySucceeds()`) that a story opts into with one call. | `src/shared/mocks/utils.ts` | `apps/demo/src/entities/article/mocks/handlers.ts`, `…/app/integration/Articles.list-request.stories.tsx` | + +## Rules + +- Every pattern in this catalog lives in the template and runs in at least + one demo page, widget, or story. An export the demo never uses does not + count as shipped. +- The reasoning lives in comments at the decision points in the code. A + reader must be able to tell what breaks if they simplify each non-obvious + part. +- Shipping a pattern includes its site entry: + `site/src/content/patterns/.md` with the problem, the decision, file + pointers, a demo link, and a "See it in the demo" section with steps that + reproduce the behavior. + +## Workflows + +### Adopting a pattern in a generated project + +1. Find the mechanism in the layer the table names, usually + `src/shared/reatom/`. Generated projects ship the same files as the + template. +2. Read the comments at the decision points before using the API. They + usually name the obvious alternative and why it fails. +3. Copy the demo wiring (`apps/demo` in the karkas repository) for your + equivalent page. + +### Adding a new pattern + +1. Read the production source in full before abstracting it. +2. Land the code in both the template and the demo, with tests and demo + wiring. +3. Add the site entry, then run the full quality gate (`mise run ci`). + +## Edge Cases + +- A pattern can land in the template before its best demo exists. The demo + link in the site entry must still point at wiring that exists today. +- If the template and demo copies of a shared file diverge, that is a bug. + Fix it before building on either copy. diff --git a/packages/create-karkas/template/messages/en/auth.json b/packages/create-karkas/template/messages/en/auth.json index adf57aba..2ddd232c 100644 --- a/packages/create-karkas/template/messages/en/auth.json +++ b/packages/create-karkas/template/messages/en/auth.json @@ -3,6 +3,9 @@ "login_description": "Use your workspace credentials to continue.", "login_email": "Email", "login_password": "Password", + "login_email_required": "Email is required", + "login_email_invalid": "Email doesn't look right", + "login_password_required": "Password is required", "login_submit": "Sign in", "login_signing_in": "Signing in", "login_error_title": "Could not sign in", diff --git a/packages/create-karkas/template/messages/es/auth.json b/packages/create-karkas/template/messages/es/auth.json index cf861411..718b8a59 100644 --- a/packages/create-karkas/template/messages/es/auth.json +++ b/packages/create-karkas/template/messages/es/auth.json @@ -3,6 +3,9 @@ "login_description": "Usa tus credenciales del espacio de trabajo para continuar.", "login_email": "Correo electrónico", "login_password": "Contraseña", + "login_email_required": "El correo electrónico es obligatorio", + "login_email_invalid": "El correo no parece válido", + "login_password_required": "La contraseña es obligatoria", "login_submit": "Iniciar sesión", "login_signing_in": "Iniciando sesión", "login_error_title": "No se pudo iniciar sesión", diff --git a/packages/create-karkas/template/mise.toml b/packages/create-karkas/template/mise.toml index f91795ea..f0f1f0c0 100644 --- a/packages/create-karkas/template/mise.toml +++ b/packages/create-karkas/template/mise.toml @@ -5,7 +5,7 @@ min_version = "2026.4.18" node = "lts" jq = "latest" vp = "latest" -hk = "latest" +hk = "1" # hk 2 cannot evaluate a config that amends the 1.x Config.pkl "npm:codemod" = "latest" [task_config] diff --git a/packages/create-karkas/template/package.json b/packages/create-karkas/template/package.json index dfbcb95d..8c331e33 100644 --- a/packages/create-karkas/template/package.json +++ b/packages/create-karkas/template/package.json @@ -85,4 +85,4 @@ "public" ] } -} \ No newline at end of file +} diff --git a/packages/create-karkas/template/src/entities/auth/mocks/handlers.ts b/packages/create-karkas/template/src/entities/auth/mocks/handlers.ts index e7f141f1..7e90d5b5 100644 --- a/packages/create-karkas/template/src/entities/auth/mocks/handlers.ts +++ b/packages/create-karkas/template/src/entities/auth/mocks/handlers.ts @@ -2,7 +2,7 @@ import { HttpResponse, delay, http } from 'msw' import { AUTH_LOGIN_API_PATH, AUTH_LOGOUT_API_PATH } from '#entities/auth/api/authApi' import { composeApiUrl } from '#shared/api' -import { to400, to500 } from '#shared/mocks/utils' +import { to400, to422, to500 } from '#shared/mocks/utils' import { authMockSession } from './data' @@ -13,6 +13,12 @@ export const authHandlers = { login: http.post(loginUrl, async ({ request }) => { await delay() const body = (await request.json()) as { email?: string; password?: string } + // A server-side uniqueness check is the one credential rule the client + // cannot pre-validate — it demonstrates a validation error that maps + // onto a form field. + if (body.email === 'taken@example.com') { + to422([{ loc: ['body', 'email'], msg: 'This email is already registered' }]) + } if (body.email !== authMockSession.user.email || body.password !== 'password') { return to400('Invalid email or password') } diff --git a/packages/create-karkas/template/src/pages/login/model/routes.test.ts b/packages/create-karkas/template/src/pages/login/model/routes.test.ts new file mode 100644 index 00000000..89eaffae --- /dev/null +++ b/packages/create-karkas/template/src/pages/login/model/routes.test.ts @@ -0,0 +1,76 @@ +import { expect, test, vi } from 'vite-plus/test' + +import { formAlertMessage } from '#shared/reatom' + +import { reatomLoginForm } from './routes' + +const json422 = (issues: Array<{ loc: Array; msg: string }>) => + new Response(JSON.stringify({ detail: issues }), { + status: 422, + headers: { 'Content-Type': 'application/json' }, + }) + +const submitAgainst = async (issues: Array<{ loc: Array; msg: string }>) => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => json422(issues)), + ) + const form = reatomLoginForm() + form.fields.email.change('alex@example.com') + form.fields.password.change('password') + await form.submit().catch(() => {}) + return form +} + +test.afterEach(() => { + vi.unstubAllGlobals() +}) + +// A 422 whose issues all name known fields is fully "handled": every message +// lives under its field, so the page-level alert must stay out. +test('a fully mapped 422 is handled', async () => { + const form = await submitAgainst([ + { loc: ['body', 'email'], msg: 'already registered' }, + { loc: ['password'], msg: 'too weak' }, + ]) + expect(form.isErrorHandled(form.submit.error())).toBe(true) + expect(form.unmappedIssues()).toHaveLength(0) + expect(formAlertMessage(form, form.isErrorHandled)).toBeNull() +}) + +// A record-level issue (no field matches `loc`) is the one failure no field +// can carry: without this, the 422 would fail silently — no field error and +// an alert that considered itself redundant. +test('a 422 with an unmapped issue is not handled', async () => { + const form = await submitAgainst([{ loc: ['record'], msg: 'sign-up disabled' }]) + expect(form.isErrorHandled(form.submit.error())).toBe(false) + expect(form.unmappedIssues()).toEqual([{ loc: ['record'], msg: 'sign-up disabled' }]) +}) + +test('a mixed 422 keeps the unmapped leftover and is not handled', async () => { + const form = await submitAgainst([ + { loc: ['body', 'email'], msg: 'already registered' }, + { loc: ['record'], msg: 'sign-up disabled' }, + ]) + expect(form.isErrorHandled(form.submit.error())).toBe(false) + expect(form.unmappedIssues()).toHaveLength(1) + // The email error is on screen, and the alert still carries the remainder. + expect(form.validation().errors).toHaveLength(1) + expect(formAlertMessage(form, form.isErrorHandled)).not.toBeNull() + expect(form.isErrorHandled({ name: 'TypeError', message: 'network down' })).toBe(false) +}) + +// Validation accepts outer whitespace; the server compares the email verbatim. +test('the submitted email is trimmed', async () => { + const fetchMock = vi.fn(async (_input: RequestInfo | URL, _init?: RequestInit) => + json422([{ loc: ['record'], msg: 'stop here' }]), + ) + vi.stubGlobal('fetch', fetchMock) + const form = reatomLoginForm() + form.fields.email.change(' alex@example.com ') + await form.submit().catch(() => {}) + + const [input, init] = fetchMock.mock.calls[0] ?? [] + const body = init?.body ?? (input instanceof Request ? await input.text() : undefined) + expect(JSON.parse(String(body))).toMatchObject({ email: 'alex@example.com' }) +}) diff --git a/packages/create-karkas/template/src/pages/login/model/routes.tsx b/packages/create-karkas/template/src/pages/login/model/routes.tsx index 7e675e32..e58c8ede 100644 --- a/packages/create-karkas/template/src/pages/login/model/routes.tsx +++ b/packages/create-karkas/template/src/pages/login/model/routes.tsx @@ -1,18 +1,71 @@ -import { urlAtom, reatomForm } from '@reatom/core' +import { reatomField, reatomForm, urlAtom, wrap } from '@reatom/core' +import { atom } from '@reatom/core' import { Fragment } from 'react' import { isAuthenticatedAtom, loginAction } from '#entities/auth' +import { m } from '#paraglide/messages.js' +import { + applyApiValidationToFields, + type ApiValidationIssue, + isApiValidationError, +} from '#shared/api' +import { withFormAutoFocusOnError, withFormSubmitHandler } from '#shared/reatom' import { createAppPath, rootRoute } from '#shared/router' import { LoginPage } from '../ui/LoginPage' const dashboardPath = createAppPath('dashboard') -const reatomLoginForm = () => - reatomForm( - { email: 'alex@example.com', password: 'password' }, - { name: 'loginForm', onSubmit: loginAction }, - ) +export const reatomLoginForm = () => { + // Issues no field claims. The page suppresses its alert only when a + // validation error is fully handled — every issue mapped onto a field. + // Unmapped or mixed responses keep the alert, otherwise a record-level 422 + // would fail silently: no field error anywhere and an alert that considers + // itself redundant. + const unmappedIssues = atom>([], 'loginForm.unmappedIssues') + + const fields = { + email: reatomField('alex@example.com', { + name: 'loginForm.email', + // Loose shape only — the backend is the source of truth for real + // address validity; this catches obviously malformed input before + // a round-trip. + validate: ({ state }) => { + const value = state.trim() + if (!value) return m.login_email_required() + if (!/^[^@\s]+@[^@\s]+\.[^@\s]+$/.test(value)) return m.login_email_invalid() + return undefined + }, + }), + password: reatomField('password', { + name: 'loginForm.password', + validate: ({ state }) => (state ? undefined : m.login_password_required()), + }), + } + + return reatomForm(fields, { + name: 'loginForm', + validateOnBlur: true, + keepErrorOnChange: false, + onSubmit: async (values) => { + try { + // Validation trims the email; the server compares it verbatim. + return await wrap(loginAction({ ...values, email: values.email.trim() })) + } catch (error) { + // Server validation issues surface under their fields; whatever no + // field claims is kept here so the page's alert can own it. + unmappedIssues.set(applyApiValidationToFields(error, fields)) + throw error + } + }, + }) + .extend(withFormSubmitHandler(), withFormAutoFocusOnError()) + .extend(() => ({ + unmappedIssues, + isErrorHandled: (error: unknown): boolean => + isApiValidationError(error) && unmappedIssues().length === 0, + })) +} export type LoginForm = ReturnType diff --git a/packages/create-karkas/template/src/pages/login/ui/LoginPage.tsx b/packages/create-karkas/template/src/pages/login/ui/LoginPage.tsx index c2c96324..d72d947f 100644 --- a/packages/create-karkas/template/src/pages/login/ui/LoginPage.tsx +++ b/packages/create-karkas/template/src/pages/login/ui/LoginPage.tsx @@ -5,11 +5,31 @@ import { bindField, reatomComponent } from '@reatom/react' import { m } from '#paraglide/messages.js' import { Alert, Button, Field, Heading, Input, Text } from '#shared/components' +import { formAlertMessage, visibleFieldError } from '#shared/reatom' import { styled } from '#styled-system/jsx' export const LoginPage = reatomComponent(({ form }: { form: LoginForm }) => { const { fields, submit } = form - const error = submit.error() + // `formAlertMessage`, not `submit.error()` directly: it stays null while a + // field-level validation owns the failure, so the alert never repeats a + // message already printed under a field. The copy here is deliberately + // canned — ApiError.message is a status string, not user-facing text. + // `isErrorHandled` from the model as the handled-predicate: it accepts a + // validation error only when every issue was mapped onto a field, so the + // alert must not re-announce those — a mapped error outlives the field + // errors it produced. Unmapped or mixed responses keep the alert. + const showErrorAlert = formAlertMessage(form, form.isErrorHandled) !== null + // Unlike ApiError.message, a 422 issue's `msg` is written for the user, so + // issues no field shows replace the canned description. + const unmappedMessages = form.unmappedIssues().map((issue) => issue.msg) + // `visibleFieldError`, not bindField's `error`: with `keepErrorOnChange: + // false` the last issue lingers in Reatom without its `triggered` flag, and + // reading that raw error would leave stale copy under the field while the + // user fixes the value. + const { error: _emailError, ...emailBind } = bindField(fields.email) + const { error: _passwordError, ...passwordBind } = bindField(fields.password) + const emailError = visibleFieldError(fields.email) + const passwordError = visibleFieldError(fields.password) const pending = !submit.ready() return ( @@ -25,34 +45,54 @@ export const LoginPage = reatomComponent(({ form }: { form: LoginForm }) => { display="flex" flexDirection="column" gap="5" - onSubmit={wrap((event) => { - event.preventDefault() - submit() - })} + // Native constraint validation (`required`, `type="email"`) would block + // the submit event before Reatom validation renders its field errors. + noValidate + onSubmit={wrap(form.handleSubmit)} > {m.login_title()} {m.login_description()} - {error && ( + {showErrorAlert && ( {m.login_error_title()} - {m.login_error_description()} + + {unmappedMessages.length > 0 + ? unmappedMessages.join(' ') + : m.login_error_description()} + )} - + {m.login_email()} - + { + fields.email.elementRef.set(element ?? undefined) + })} + type="email" + autoComplete="email" + {...emailBind} + /> + {emailError && {emailError}} - + {m.login_password()} - + { + fields.password.elementRef.set(element ?? undefined) + })} + type="password" + autoComplete="current-password" + {...passwordBind} + /> + {passwordError && {passwordError}}