Deprecate events passed to enhance?

[gbouteiller]

• [✓] Before posting an issue, read the FAQ and search the previous issues.

Description
When events are added to use:enhance, if the form element is created and destroyed multiple times (for example, the form is in a sheet or dialog or, even simpler, in an if like in the MRE below), the events are pushed multiple times to formEvents but never destroyed.

MRE
In that example, if you toggle the form multiple times, you can see that onUpdated is called multiple times

Possible solution
Instead of returning kitEnhance here :

sveltekit-superforms/src/lib/client/superForm.ts

Line 1605 in 1b7a158

return kitEnhance(FormElement, async (submitParams) => {

it could be something like this :

const enhance = kitEnhance("...");
return {
  destroy: () => {
    // same as in onDestroy()
    for (const events of Object.values(formEvents)) {
      events.length = 0;
    }
    // call native enhance destroy
    enhance.destroy();
  }
}

What are your thoughts?
By the way, thank you again for this awesome library.

[ciscoheat]

Thank you, will take a look and hopefully it will be included in the next release. Glad you like Superforms. :)

[ciscoheat]

I've added a fix for the next release. Do you have any good reason not to put the event(s) directly in the options when calling superForm? I'm considering deprecating this "add events in enhance" feature, as I have never found a really good use case for it.

[gbouteiller]

I'm creating an app with crud functionalities.
I have a generic page as a component for each collection where generic events are passed to the enhance action of a superform passed as a prop and instantiated in the specific page (see below).

components/ui/items-page.svelte (simplified version)

<script lang="ts" generics="Item extends {id: string}, Upsert extends Record<string, any>">
  let {isAdmin, items, sfDelete, sfUpsert, UpsertFields}: ItemsPageProps<Item, Upsert> = $props()
  let {delayed: delayedDelete, enhance: enhanceDelete, form: formDelete, message: messageDelete, submitting: submittingDelete} = sfDelete
  let {delayed: delayedUpsert, enhance: enhanceUpsert, message: messageUpsert, reset, submitting: submittingUpsert} = sfUpsert

  const onUpdated = () => toast.success("...")
</script>

<div>
  {#if isAdmin}
    <div>{@render UpsertSheet()}</div>
  {/if}
  ...
</div>

{#snippet UpsertSheet()}
  <Sheet.Root>
    <Sheet.Trigger>Create</Sheet.Trigger>
    <Sheet.Content>
      <form method="post" action="?/upsert" use:enhanceUpsert={{onUpdated}}>
        {@render UpsertFields()}
        <Form.Button>Submit</Form.Button>
      </form>
    </Sheet.Content>
  </Sheet.Root>
{/snippet}

For each collection like projects, I create a page where I instantiate the superforms for upsert and delete. The instantiation is done here to pass the upsert superform to the FormSnap components and to bind to field values

projects/+page.svelte (simplified version)

<script lang="ts">
  let {data}: {data: PageData} = $props()
  let {isAdmin, items} = $derived(data)

  let sfDelete = superForm(data.svDelete, {validators: zodClient(deleteSchema)})
  let sfUpsert = superForm(data.svUpsert, {validators: zodClient(projectUpsertSchema)})
  let {form} = sfUpsert
</script>

<ItemsPage {isAdmin} {items} {sfDelete} {sfUpsert}>
  {#snippet UpsertFields()}
    <input type="hidden" name="id" value={$form.id} />
    <Form.Field form={sfUpsert} name="name">
      <Form.Control let:attrs>
        <Form.Label>Nom</Form.Label>
        <Input {...attrs} bind:value={$form.name} />
      </Form.Control>
      <Form.FieldErrors />
    </Form.Field>
  {/snippet}
</CollectionItemsPage>

I have tried to instantiate the superform directly in the generic component and pass its $form store to the specific page via a snippet param but I lose reactivity.

[ciscoheat]

I'm not sure I understand still, since onUpdated looks like a simple toast notification (maybe you've simplified it), what prevents you from adding it directly?

let sfUpsert = superForm(data.svUpsert, {
  validators: zodClient(projectUpsertSchema),
  onUpdated: () => toast.success("...")
})

[gbouteiller]

As you said , I simplified it 🙂. In fact, I use it for multiple interactions on the generic page (closing the sheet, updating some data, etc...). But even if it was a simple toast, I want the toast to be on the generic page as I don't want, if I have 10 collections, to define the same behavior 10 times (hence the generic crud component 🙂).

[ciscoheat]

Right, but if you use it in multiple locations, it will append the events to the others as well, and there are internal state that may cause problems too. That's why I'm not sure it's the best to keep. It's was a very early feature.