Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a genric type for "file" fields (same as "json" ?) #65

Open
ppierre opened this issue May 25, 2023 · 7 comments
Open

Add a genric type for "file" fields (same as "json" ?) #65

ppierre opened this issue May 25, 2023 · 7 comments

Comments

@ppierre
Copy link

ppierre commented May 25, 2023

Thanks for such a nice and straight forward tool.

I was testing file upload with PocketBase/Vue/FormKit. I managed to strongly type my form.

type FormKitS<T> = FormKitSchemaFormKit & {
  name: keyof T
}
const nvlMaison = ref<MaisonResponse>()

const schema: FormKitS<MaisonRecord>[] = [ /*...*/ ]

But while processing my form, I had to "patch" the type:

/* "Patch" `images` BC not `string[]` but `{ file: File; name: string }[]` */
export type MaisonResponseWithFiles = {
  images?: {
    file: File
    name: string
  }[]
} & MaisonResponse

FormKit sends me the files property of the form field. Not the string[] who would exist only after PocketBase had processed the images.

My idea was of generating for file fields, a generic type with a default value:

export type MaisonRecord<Timages = string[]> = {
  images?: Timages;
  /*...*/
};

Somehow like json fields.

Or someone can find a better solution where the alternate type is fixed or specified only once for all file's fields ?

I am suggesting in reason for this frequent use case. Feel free to close the issue : the current situation is very usable thanks to your nice generated types 👍

@ppierre
Copy link
Author

ppierre commented May 25, 2023

Well, I have "solved" it with some shameless duck typing:

/**
 * process form data to append files
 * @param data form data to process
 * @returns FormData object with files
 */
function processFiles(data:Object) {
  const formData = new FormData()
  Object.entries(data).forEach(([key, value]) => {
    if (value?.[0]?.file instanceof File) {
      value.forEach((file) => {
        formData.append(key, file.file)
      })  
    } else {
      formData.append(key, value as string)
    }
  })
  return formData
}

@patmood
Copy link
Owner

patmood commented May 26, 2023

I haven't used file fields in pocketbase so I'll need to do more investigation.

One thing worth mentioning is that there should be a MaisonRecord that is better suited to creating records (as opposed to MaisonResponse). It will preserve the optional fields and be missing the system fields like id/created/updated.

Do all file create requests have the type { file: File; name: string }[]? Perhaps that should just be typed out of the box.

@ppierre
Copy link
Author

ppierre commented May 27, 2023

For Record vs. Response, I intend to a strategy successfully used with SupaBase: a unique form component connected to an upsert method. And will use it for a new page and update page. It works quite well: no ID => new record ; an ID => update record. Formkit help by silently passing the ID field if provided at the creation of the form. (And delete button shown on ID detection.)

PocketBase upset me by not having an upsert method (pun intended). But I should be able to write a generic one as you provide mapping for Collection/type/Id/string...

Does { file: File; name: string }[] is universal ? (NO)

I was asking myself the same question before opening the issue. I have looked further: and it seems not.

What seems universal is watching the <input type="file"> ; grabbing his files:File[] property on change and using it as value SO Formik, file handling.

FormKit by extracting the name of File seem to do a sensible thing. But it isn't an obligation.

Upon reflection, IMHO, having an union should work quite well.

  • Existing record, you have string[]:
    use PB API to display the thumbnails (and a delete images button).
  • Empty value or Files[] (or { file:File; name:string }[]):
    use FileReader or URL.createObjectURL to display a preview of the images.

One could make a component who accept both type (string[] and Files[] or like) and work accordingly. Like my form strategy with upsert.

I have just started using form with PocketBase. I will make a complete example by September. So no hurry for me.

In my dream PocketBase will have upsert and process automatically File values under the key containing them (key:File, key:File[], key:{ file:File)[] ...) (Jocking)

So maybe others will chip in. but for me being able to change the type associated with file input could help ; should it be a File[] or derivative or a union (string[] | File[]) ? Sadly, it doesn't seem to be universal. FromKit use { file:File; name:string }[] maybe because he displays the list of files... Who know? If the type were : string[] | File[], I would try to write a custom field to wrap Formkit original one to provide adequate value.

@ppierre
Copy link
Author

ppierre commented May 27, 2023

string | File[] and string[] | File[] should be good.

I managed to convert "FormKit field input" to what React people do:

type FormKitS<T> = FormKitSchemaFormKit &
  (
    | {
        name: keyof T
      }
    | {
        ignore: true
      }
  )
const nvlMaison = ref<MaisonResponse>()

const schema: FormKitS<MaisonRecord>[] = [
  /* ... */
  {
    $formkit: 'file',
    ignore: true,
    label: 'Images',
    accept: '.png,.jpg,.jpeg,.bmp,.gif,.tiff',
    multiple: 'true',
    onchange: (e: Event) => {
      const target = e.target as HTMLInputElement
      const files = target.files
      if (files) {
        const images = Array.from(files)
        nvlMaison.value = {
          ...nvlMaison.value,
          // @ts-ignore Impossible to assign type `File[]` to type `string[]`
          images
        }
      }
    }
  }
]

And get rid of the file.file...

function processFiles(data: Object) {
  const formData = new FormData()
  Object.entries(data).forEach(([key, value]) => {
    if (value?.[0] instanceof File) {
      value.forEach((file) => {
        formData.append(key, file)
      })
    } else {
      formData.append(key, value as string)
    }
  })
  return formData
}

I will ask FormKit if they are willing to optionally return File[] instead of { file:File; name:string }[] for value of file input.

@patmood
Copy link
Owner

patmood commented Aug 27, 2023

Hey @ppierre do the latest changes to the pocketbase SDK help this issue? See the notes for v0.17

You can now send the file as an object

@ppierre
Copy link
Author

ppierre commented Aug 27, 2023

@patmood thanks for the tip.
But sadly, the native form element gives a FileList (sort of array of File[].
And FormKit uses a bastardized version : { file:File; name:string }[]

It still exists a discrepancy between:

  • database: string or string[]
  • form field: always an array,FileList (File[]) (or FormKit { file:File; name:string }[]
  • upload: multipart and now File (and does File[]and FileList work ?)

@ppierre
Copy link
Author

ppierre commented Aug 27, 2023

After reflection,
You should change the type of (single) image field to string | File . But I don't know for multiple images ? (string[] | File[] | (string|File)[] | FileList ?)

For my code I should make a custom field component who accept string | File as value. I have already done this for Vue/FormKit/Supabase. At first, I haven't done it for PocketBase ; thinking it integrates image management, so why the complexity.
But systematically making custom components for uploading and displaying images: it seems a good practice !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants