From a6c277bb4649489324999b07863d55ee7e4ece0f Mon Sep 17 00:00:00 2001 From: Edmund Hung Date: Sun, 25 Feb 2024 14:45:31 +0100 Subject: [PATCH] fix: preserve files only on client --- packages/conform-dom/formdata.ts | 35 +- packages/conform-dom/submission.ts | 7 +- playground/app/routes/metadata.tsx | 51 ++- tests/integrations/metadata.spec.ts | 483 ++++++++++++++++++++++++++-- 4 files changed, 527 insertions(+), 49 deletions(-) diff --git a/packages/conform-dom/formdata.ts b/packages/conform-dom/formdata.ts index 65ff8124..58860ec6 100644 --- a/packages/conform-dom/formdata.ts +++ b/packages/conform-dom/formdata.ts @@ -165,22 +165,28 @@ export function isFile(obj: unknown): obj is File { * Normalize value by removing empty object or array, empty string and null values */ export function normalize>( - value: Type | null, -): Type | null | undefined; + value: Type, + acceptFile?: boolean, +): Type | undefined; export function normalize>( - value: Type | null, -): Type | null | undefined; -export function normalize(value: unknown): unknown | undefined; + value: Type, + acceptFile?: boolean, +): Type | undefined; +export function normalize( + value: unknown, + acceptFile?: boolean, +): unknown | undefined; export function normalize< Type extends Record | Array, >( - value: Type | null, -): Record | Array | null | undefined { + value: Type, + acceptFile = true, +): Record | Array | undefined { if (isPlainObject(value)) { const obj = Object.keys(value) .sort() .reduce>((result, key) => { - const data = normalize(value[key]); + const data = normalize(value[key], acceptFile); if (typeof data !== 'undefined') { result[key] = data; @@ -201,26 +207,17 @@ export function normalize< return undefined; } - return value.map(normalize); + return value.map((item) => normalize(item, acceptFile)); } if ( (typeof value === 'string' && value === '') || value === null || - (isFile(value) && value.size === 0) + (isFile(value) && (!acceptFile || value.size === 0)) ) { return; } - // We will skip serializing file if the result is sent to the client - if (isFile(value)) { - return Object.assign(value, { - toJSON() { - return; - }, - }); - } - return value; } diff --git a/packages/conform-dom/submission.ts b/packages/conform-dom/submission.ts index 7acc6791..47b3c44e 100644 --- a/packages/conform-dom/submission.ts +++ b/packages/conform-dom/submission.ts @@ -278,7 +278,12 @@ export function replySubmission( return { status: context.intent ? undefined : error ? 'error' : 'success', intent: context.intent ? context.intent : undefined, - initialValue: normalize(context.payload) ?? {}, + initialValue: + normalize( + context.payload, + // We can't serialize the file and send it back from the server, but we can preserve it in the client + typeof document !== 'undefined', + ) ?? {}, error, state: context.state, fields: Array.from(context.fields), diff --git a/playground/app/routes/metadata.tsx b/playground/app/routes/metadata.tsx index 3c614ae5..442887eb 100644 --- a/playground/app/routes/metadata.tsx +++ b/playground/app/routes/metadata.tsx @@ -27,8 +27,33 @@ const schema = z.object({ bookmarks.length, 'Bookmark URLs are repeated', ), + file: z.instanceof(File, { message: 'File is required' }), + files: z + .instanceof(File) + .array() + .min(1, 'At least 1 file is required') + .refine( + (files) => files.every((file) => file.type === 'application/json'), + 'Only JSON file is accepted', + ), }); +const getPrintableValue = (value: unknown) => { + if (typeof value === 'undefined') { + return; + } + + return JSON.parse( + JSON.stringify(value, (key, value) => { + if (value instanceof File) { + return `${value.name} (${value.size} bytes)`; + } + + return value; + }), + ); +}; + export async function loader({ request }: LoaderArgs) { const url = new URL(request.url); @@ -62,14 +87,14 @@ export default function Example() { const bookmarks = fields.bookmarks.getFieldList(); return ( -
+ @@ -128,6 +169,12 @@ export default function Example() { ); })} + + + + + +
); diff --git a/tests/integrations/metadata.spec.ts b/tests/integrations/metadata.spec.ts index 696e98b6..9cd7de71 100644 --- a/tests/integrations/metadata.spec.ts +++ b/tests/integrations/metadata.spec.ts @@ -16,10 +16,36 @@ function getFieldset(form: Locator) { url: list.nth(1).locator('[name="bookmarks[1].url"]'), }, ], + file: form.locator('[name="file"]'), + files: form.locator('[name="files"]'), }; } -async function validateMetadata(page: Page, noJS?: boolean) { +const textFile = { + name: 'example.txt', + mimeType: 'text/plain', + buffer: Buffer.from('hello world'), +}; + +const jsonFile = { + name: 'example.json', + mimeType: 'application/json', + buffer: Buffer.from(JSON.stringify({ a: 'foo', b: 123, c: false })), +}; + +const yamlFile = { + name: 'example.yaml', + mimeType: 'application/x-yaml', + buffer: Buffer.from(['a: foo', 'b: 123', 'c: false'].join('\n')), +}; + +async function validateMetadata( + page: Page, + options: { + noJS?: boolean; + clientValidate?: boolean; + } = {}, +) { const playground = getPlayground(page); const fieldset = getFieldset(playground.container); @@ -61,6 +87,16 @@ async function validateMetadata(page: Page, noJS?: boolean) { valid: true, allErrors: {}, }, + file: { + allErrors: {}, + dirty: false, + valid: true, + }, + files: { + allErrors: {}, + dirty: false, + valid: true, + }, }); await playground.submit.click(); @@ -82,6 +118,8 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[0].url': ['Url is required'], 'bookmarks[1].name': ['Name is required'], 'bookmarks[1].url': ['Url is required'], + file: ['File is required'], + files: ['At least 1 file is required'], }, }, title: { @@ -119,12 +157,28 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[1].url': ['Url is required'], }, }, + file: { + errors: ['File is required'], + allErrors: { + file: ['File is required'], + }, + dirty: false, + valid: false, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, }); // To confirm if dirty check works correctly await fieldset.title.fill(''); await fieldset.title.fill('Test'); - if (!noJS) { + if (!options.noJS) { await expect.poll(playground.result).toEqual({ form: { status: 'error', @@ -143,6 +197,8 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[0].url': ['Url is required'], 'bookmarks[1].name': ['Name is required'], 'bookmarks[1].url': ['Url is required'], + file: ['File is required'], + files: ['At least 1 file is required'], }, }, title: { @@ -180,11 +236,27 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[1].url': ['Url is required'], }, }, + file: { + errors: ['File is required'], + allErrors: { + file: ['File is required'], + }, + dirty: false, + valid: false, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, }); } await fieldset.title.fill('Projects'); - if (!noJS) { + if (!options.noJS) { await expect.poll(playground.result).toEqual({ form: { status: 'error', @@ -203,6 +275,8 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[0].url': ['Url is required'], 'bookmarks[1].name': ['Name is required'], 'bookmarks[1].url': ['Url is required'], + file: ['File is required'], + files: ['At least 1 file is required'], }, }, title: { @@ -240,6 +314,22 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[1].url': ['Url is required'], }, }, + file: { + errors: ['File is required'], + allErrors: { + file: ['File is required'], + }, + dirty: false, + valid: false, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, }); } @@ -248,7 +338,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { form: { status: 'error', initialValue: { - title: noJS ? 'Projects' : 'Test', + title: options.noJS ? 'Projects' : 'Test', bookmarks: [null, null], }, value: { @@ -262,10 +352,12 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[0].url': ['Url is required'], 'bookmarks[1].name': ['Name is required'], 'bookmarks[1].url': ['Url is required'], + file: ['File is required'], + files: ['At least 1 file is required'], }, }, title: { - initialValue: noJS ? 'Projects' : 'Test', + initialValue: options.noJS ? 'Projects' : 'Test', value: 'Projects', dirty: true, valid: true, @@ -299,6 +391,22 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[1].url': ['Url is required'], }, }, + file: { + errors: ['File is required'], + allErrors: { + file: ['File is required'], + }, + dirty: false, + valid: false, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, }); await fieldset.bookmarks[0].name.fill('Conform'); @@ -306,7 +414,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { await expect.poll(playground.result).toEqual({ form: { status: 'error', - initialValue: noJS + initialValue: options.noJS ? { title: 'Projects', bookmarks: [ @@ -335,17 +443,19 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[0].url': ['Url is required'], 'bookmarks[1].name': ['Name is required'], 'bookmarks[1].url': ['Url is required'], + file: ['File is required'], + files: ['At least 1 file is required'], }, }, title: { - initialValue: noJS ? 'Projects' : 'Test', + initialValue: options.noJS ? 'Projects' : 'Test', value: 'Projects', dirty: true, valid: true, allErrors: {}, }, bookmarks: { - initialValue: noJS + initialValue: options.noJS ? [ { name: 'Conform', @@ -368,7 +478,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { }, }, 'bookmarks[0]': { - initialValue: noJS ? { name: 'Conform' } : undefined, + initialValue: options.noJS ? { name: 'Conform' } : undefined, value: { name: 'Conform', }, @@ -386,6 +496,22 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[1].url': ['Url is required'], }, }, + file: { + errors: ['File is required'], + allErrors: { + file: ['File is required'], + }, + dirty: false, + valid: false, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, }); await fieldset.bookmarks[0].url.fill('https://conform.guide'); @@ -393,7 +519,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { await expect.poll(playground.result).toEqual({ form: { status: 'error', - initialValue: noJS + initialValue: options.noJS ? { title: 'Projects', bookmarks: [ @@ -423,17 +549,19 @@ async function validateMetadata(page: Page, noJS?: boolean) { allErrors: { 'bookmarks[1].name': ['Name is required'], 'bookmarks[1].url': ['Url is required'], + file: ['File is required'], + files: ['At least 1 file is required'], }, }, title: { - initialValue: noJS ? 'Projects' : 'Test', + initialValue: options.noJS ? 'Projects' : 'Test', value: 'Projects', dirty: true, valid: true, allErrors: {}, }, bookmarks: { - initialValue: noJS + initialValue: options.noJS ? [ { name: 'Conform', @@ -457,7 +585,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { }, }, 'bookmarks[0]': { - initialValue: noJS + initialValue: options.noJS ? { name: 'Conform', url: 'https://conform.guide' } : undefined, value: { @@ -476,6 +604,22 @@ async function validateMetadata(page: Page, noJS?: boolean) { 'bookmarks[1].url': ['Url is required'], }, }, + file: { + errors: ['File is required'], + allErrors: { + file: ['File is required'], + }, + dirty: false, + valid: false, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, }); await fieldset.bookmarks[1].name.fill('Super cool website'); @@ -484,7 +628,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { await expect.poll(playground.result).toEqual({ form: { status: 'error', - initialValue: noJS + initialValue: options.noJS ? { title: 'Projects', bookmarks: [ @@ -519,17 +663,19 @@ async function validateMetadata(page: Page, noJS?: boolean) { valid: false, allErrors: { bookmarks: ['Bookmark URLs are repeated'], + file: ['File is required'], + files: ['At least 1 file is required'], }, }, title: { - initialValue: noJS ? 'Projects' : 'Test', + initialValue: options.noJS ? 'Projects' : 'Test', value: 'Projects', dirty: true, valid: true, allErrors: {}, }, bookmarks: { - initialValue: noJS + initialValue: options.noJS ? [ { name: 'Conform', @@ -559,7 +705,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { }, }, 'bookmarks[0]': { - initialValue: noJS + initialValue: options.noJS ? { name: 'Conform', url: 'https://conform.guide' } : undefined, value: { @@ -571,7 +717,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { allErrors: {}, }, 'bookmarks[1]': { - initialValue: noJS + initialValue: options.noJS ? { name: 'Super cool website', url: 'https://conform.guide' } : undefined, value: { @@ -582,14 +728,155 @@ async function validateMetadata(page: Page, noJS?: boolean) { valid: true, allErrors: {}, }, + file: { + errors: ['File is required'], + allErrors: { + file: ['File is required'], + }, + dirty: false, + valid: false, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, }); await fieldset.bookmarks[1].url.fill('https://remix.guide'); await playground.submit.click(); await expect.poll(playground.result).toEqual({ form: { - status: 'success', - initialValue: noJS + status: 'error', + initialValue: options.noJS + ? { + title: 'Projects', + bookmarks: [ + { + name: 'Conform', + url: 'https://conform.guide', + }, + { + name: 'Super cool website', + url: 'https://remix.guide', + }, + ], + } + : { + title: 'Test', + bookmarks: [null, null], + }, + value: { + title: 'Projects', + bookmarks: [ + { + name: 'Conform', + url: 'https://conform.guide', + }, + { + name: 'Super cool website', + url: 'https://remix.guide', + }, + ], + }, + dirty: true, + valid: false, + allErrors: { + file: ['File is required'], + files: ['At least 1 file is required'], + }, + }, + title: { + initialValue: options.noJS ? 'Projects' : 'Test', + value: 'Projects', + dirty: true, + valid: true, + allErrors: {}, + }, + bookmarks: { + initialValue: options.noJS + ? [ + { + name: 'Conform', + url: 'https://conform.guide', + }, + { + name: 'Super cool website', + url: 'https://remix.guide', + }, + ] + : [null, null], + value: [ + { + name: 'Conform', + url: 'https://conform.guide', + }, + { + name: 'Super cool website', + url: 'https://remix.guide', + }, + ], + dirty: true, + valid: true, + allErrors: {}, + }, + 'bookmarks[0]': { + initialValue: options.noJS + ? { name: 'Conform', url: 'https://conform.guide' } + : undefined, + value: { + name: 'Conform', + url: 'https://conform.guide', + }, + dirty: true, + valid: true, + allErrors: {}, + }, + 'bookmarks[1]': { + initialValue: options.noJS + ? { name: 'Super cool website', url: 'https://remix.guide' } + : undefined, + value: { + name: 'Super cool website', + url: 'https://remix.guide', + }, + dirty: true, + valid: true, + allErrors: {}, + }, + file: { + errors: ['File is required'], + allErrors: { + file: ['File is required'], + }, + dirty: false, + valid: false, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, + }); + + // This is only tested with client validation because only it will returns the file after parsing the submission + // TODO: handle this with server validation as well as long as JS is enabled? + if (!options.clientValidate) { + return; + } + + await fieldset.file.setInputFiles(textFile); + await playground.submit.click(); + await expect.poll(playground.result).toEqual({ + form: { + status: 'error', + initialValue: options.noJS ? { title: 'Projects', bookmarks: [ @@ -619,20 +906,141 @@ async function validateMetadata(page: Page, noJS?: boolean) { url: 'https://remix.guide', }, ], + file: 'example.txt (11 bytes)', }, dirty: true, + valid: false, + allErrors: { + files: ['At least 1 file is required'], + }, + }, + title: { + initialValue: options.noJS ? 'Projects' : 'Test', + value: 'Projects', + dirty: true, + valid: true, + allErrors: {}, + }, + bookmarks: { + initialValue: options.noJS + ? [ + { + name: 'Conform', + url: 'https://conform.guide', + }, + { + name: 'Super cool website', + url: 'https://remix.guide', + }, + ] + : [null, null], + value: [ + { + name: 'Conform', + url: 'https://conform.guide', + }, + { + name: 'Super cool website', + url: 'https://remix.guide', + }, + ], + dirty: true, valid: true, allErrors: {}, }, + 'bookmarks[0]': { + initialValue: options.noJS + ? { name: 'Conform', url: 'https://conform.guide' } + : undefined, + value: { + name: 'Conform', + url: 'https://conform.guide', + }, + dirty: true, + valid: true, + allErrors: {}, + }, + 'bookmarks[1]': { + initialValue: options.noJS + ? { name: 'Super cool website', url: 'https://remix.guide' } + : undefined, + value: { + name: 'Super cool website', + url: 'https://remix.guide', + }, + dirty: true, + valid: true, + allErrors: {}, + }, + file: { + value: 'example.txt (11 bytes)', + dirty: true, + valid: true, + allErrors: {}, + }, + files: { + errors: ['At least 1 file is required'], + allErrors: { + files: ['At least 1 file is required'], + }, + dirty: false, + valid: false, + }, + }); + + await fieldset.files.setInputFiles([jsonFile, yamlFile]); + await playground.submit.click(); + await expect.poll(playground.result).toEqual({ + form: { + status: 'error', + initialValue: options.noJS + ? { + title: 'Projects', + bookmarks: [ + { + name: 'Conform', + url: 'https://conform.guide', + }, + { + name: 'Super cool website', + url: 'https://remix.guide', + }, + ], + } + : { + title: 'Test', + bookmarks: [null, null], + }, + value: { + title: 'Projects', + bookmarks: [ + { + name: 'Conform', + url: 'https://conform.guide', + }, + { + name: 'Super cool website', + url: 'https://remix.guide', + }, + ], + file: 'example.txt (11 bytes)', + files: ['example.json (29 bytes)', 'example.yaml (22 bytes)'], + }, + dirty: true, + valid: false, + allErrors: { + files: ['Only JSON file is accepted'], + }, + }, title: { - initialValue: noJS ? 'Projects' : 'Test', + initialValue: options.noJS ? 'Projects' : 'Test', value: 'Projects', dirty: true, valid: true, allErrors: {}, }, bookmarks: { - initialValue: noJS + initialValue: options.noJS ? [ { name: 'Conform', @@ -659,7 +1067,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { allErrors: {}, }, 'bookmarks[0]': { - initialValue: noJS + initialValue: options.noJS ? { name: 'Conform', url: 'https://conform.guide' } : undefined, value: { @@ -671,7 +1079,7 @@ async function validateMetadata(page: Page, noJS?: boolean) { allErrors: {}, }, 'bookmarks[1]': { - initialValue: noJS + initialValue: options.noJS ? { name: 'Super cool website', url: 'https://remix.guide' } : undefined, value: { @@ -682,18 +1090,37 @@ async function validateMetadata(page: Page, noJS?: boolean) { valid: true, allErrors: {}, }, + file: { + value: 'example.txt (11 bytes)', + dirty: true, + valid: true, + allErrors: {}, + }, + files: { + value: ['example.json (29 bytes)', 'example.yaml (22 bytes)'], + dirty: true, + valid: false, + errors: ['Only JSON file is accepted'], + allErrors: { + files: ['Only JSON file is accepted'], + }, + }, }); } test.describe('With JS', () => { test('Client Validation', async ({ page }) => { await page.goto('/metadata'); - await validateMetadata(page); + await validateMetadata(page, { + clientValidate: true, + }); }); test('Server Validation', async ({ page }) => { await page.goto('/metadata?noClientValidate=yes'); - await validateMetadata(page); + await validateMetadata(page, { + clientValidate: false, + }); }); }); @@ -702,6 +1129,8 @@ test.describe('No JS', () => { test('Validation', async ({ page }) => { await page.goto('/metadata'); - await validateMetadata(page, true); + await validateMetadata(page, { + noJS: true, + }); }); });