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

Opt-out of data-svelte-h #10992

Closed
hyunbinseo opened this issue Nov 7, 2023 · 4 comments
Closed

Opt-out of data-svelte-h #10992

hyunbinseo opened this issue Nov 7, 2023 · 4 comments

Comments

@hyunbinseo
Copy link
Contributor

hyunbinseo commented Nov 7, 2023

Describe the bug

In the following example, even if the form's default action returns { date: new Date() }

  • formArray is updated and therefore logged in the console.
  • <button> element is not recreated, since it has a data-svelte-h attribute.
  • <button> element stays disabled.
<script lang="ts">
  import { enhance } from '$app/forms';

  export let form;

  $: formArray = [form];
  $: console.log(formArray);
</script>

<form
  method="post"
  use:enhance
  on:submit={(e) => {
    if (e.submitter instanceof HTMLButtonElement) e.submitter.disabled = true;
  }}
>
  {#if !form || form.date}
    {#each formArray as _}
      <button>Button</button>
    {/each}
  {/if}
</form>

Improve hydration speed by adding data-svelte-h attribute to detect unchanged HTML elements (sveltejs/svelte#7426)

Reproduction

Reference the StackBlitz project.

The reproduction's goal is to disable the submit button until the form action responds, thus blocking double submit.

  1. Press Fail1 button. - Fail1 stays disabled.
  2. Press Fail2 button.
  3. Press Fail3 button. - Fail3 button is recreated and can be pressed.
// +page.server.ts

import { fail } from '@sveltejs/kit';

export const actions = {
  message: () => fail(400, { message: 'Hello World' }),
  date: () => fail(400, { date: new Date() }),
};
<!-- +page.svelte -->

<script lang="ts">
  import { enhance } from '$app/forms';
  import type { Action } from 'svelte/action';

  export let form;

  const action: Action = (node) => {
    if (node instanceof HTMLButtonElement) {
      console.log(`${node.textContent} button is mounted.`);
      console.log(node);
    }
  };

  $: formArray = [form];
  $: console.log(formArray);
</script>

<form
  method="post"
  use:enhance
  on:submit={(e) => {
    if (e.submitter instanceof HTMLButtonElement) {
      console.log(`${e.submitter.textContent} button is disabled.`);
      e.submitter.disabled = true;
    }
  }}
>
  {#if !form || form.date}
    {#each formArray as _}
      <button use:action formaction="?/date">Fail1</button>
    {/each}
    <button use:action formaction="?/message">Fail2</button>
  {:else}
    {#if form}
      <button use:action formaction="?/message">Fail3</button>
    {/if}
  {/if}
</form>

Logs

Array [ null ]
Fail1 button is mounted. 
<button formaction="?/date" data-svelte-h="svelte-1nb5vgx">
Fail2 button is mounted. 
<button formaction="?/message" data-svelte-h="svelte-j1ksp3">

### Press Fail1 button #####################################
Fail1 button is disabled. 
Array [ {…} ]

### Press Fail2 button #####################################
Fail2 button is disabled. 
Array [ null ]
Array [ {…} ]
Fail3 button is mounted. 
<button formaction="?/message">

### Press Fail3 button #####################################
Fail3 button is disabled. 
Array [ null ]
Fail1 button is mounted. 
<button formaction="?/date">
Fail2 button is mounted. 
<button formaction="?/message">
Array [ {…} ]
Fail3 button is mounted. 
<button formaction="?/message">

System Info

Binaries:
  Node: 18.18.0 - /usr/local/bin/node
  Yarn: 1.22.19 - /usr/local/bin/yarn
  npm: 9.4.2 - /usr/local/bin/npm
  pnpm: 8.9.2 - /usr/local/bin/pnpm
npmPackages:
  @sveltejs/adapter-auto: ^2.1.0 => 2.1.1 
  @sveltejs/kit: ^1.27.2 => 1.27.3 
  svelte: ^4.2.2 => 4.2.2 
  vite: ^4.5.0 => 4.5.0

Severity

annoyance

Additional Information

  • I would love to manually disable data-svelte-h attributes.
  • {#if form} seems to be enough to recreate Fail3 button.
@dummdidumm
Copy link
Member

dummdidumm commented Nov 7, 2023

This isn't related to data-svelte-h. It's related to the fix of this bug, since which we set the form property to null before setting it to the actual value. In the initial request this does only triggers one rerun since the previous value was null, too, but all subsequent form submissions cause two updates to form. That's why the first time you see the button as disabled, but not the times after afterwards, because the if+each combination results in the lists getting destroyed and recreated.
What's the use case for relying on the specific dom modification behavior?

@hyunbinseo
Copy link
Contributor Author

I thought it was a data-svelte-h issue, since I worked around as follows, without a each block.

<!-- src/routes/temp/+page.svelte -->

<script lang="ts">
  import { enhance } from '$app/forms';
  export let form;
  $: console.log(form);
</script>

<form
  method="post"
  use:enhance
  on:submit={(e) => {
    if (e.submitter instanceof HTMLButtonElement) e.submitter.disabled = true;
  }}
>
  {#if !form || form.date}
    {#if !form}
      <!-- Static element with `data-svelte-h` attribute. -->
      <button>Submit</button>
    {:else}
      <button>Submit</button>
    {/if}
  {/if}
</form>
// +page.server.ts
export const actions = {
  default: () => ({ date: new Date() }),
};

And yes, I newly learned that the form property is set to null then to the actual value.

# Initial load
null

# First Click
Object { date: Date Tue Nov 07 2023 17:50:54 GMT+0900 (대한민국 표준시) }

# Second Click
- null
- Object { date: Date Tue Nov 07 2023 17:50:57 GMT+0900 (대한민국 표준시) }

Even with this knowledge, I am having a hard time understanding the o.g. reproduction.

### Press Fail1 button #####################################
Fail1 button is disabled. 
Array [ {…} ] -- formArray is updated, therefore logged in the console

Why doesn't the following each block re-run, despite an updated formArray reactive statement?

{#each formArray as _}
  <button use:action formaction="?/date">Fail1</button>
{/each}

The only case I'm using this is to block double submission under JavaScript.

@dummdidumm
Copy link
Member

It's not doing it the first time because the form is null in the beginning, and setting it to null again does nothing.

@hyunbinseo
Copy link
Contributor Author

If this is not a bug but an intended behavior, this issue can be closed.

I am not sure what to rename the title to. It seems misleading.


The intention of this issue was to workaround the following workaround.

<script lang="ts">
  import { enhance } from '$app/forms';

  export let form;

  // Restrict form double submit by disabling the element that sent the submit event.
  // Reference https://developer.mozilla.org/en-US/docs/Web/API/SubmitEvent/submitter
  const disableSubmitter = (e: SubmitEvent) => {
    if (e.submitter instanceof HTMLButtonElement || e.submitter instanceof HTMLInputElement) {
      e.submitter.disabled = true;
    }
  };
</script>

<form method="post" use:enhance on:submit={disableSubmitter}>
  <!-- Remove the disabled submit button and recreate one afterwards. -->
  {#if !form}
    <button>Submit</button>
  {:else}
    <button>Submit</button>
  {/if}
</form>

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