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

[1.x] useForm wrongly overwrites default values ​​after successful submission #1935

Merged
merged 4 commits into from
Sep 21, 2024

Conversation

WillRy
Copy link
Contributor

@WillRy WillRy commented Jul 27, 2024

reset from useForm is not functional after a successful submission. This is due to the form's internal onSuccess hook setting defaults to the current form state (line 172).

The bug happens in any version above 1.0.14 in vue3

Look at line (172).

onSuccess: async (page) => {
this.processing = false
this.progress = null
this.clearErrors()
this.wasSuccessful = true
this.recentlySuccessful = true
recentlySuccessfulTimeoutId = setTimeout(() => (this.recentlySuccessful = false), 2000)
const onSuccess = options.onSuccess ? await options.onSuccess(page) : null
defaults = cloneDeep(this.data())
this.isDirty = false
return onSuccess
},

Whem we look at the source code of React and Svelte drivers, we dont see this behaviour. Older versions of the Vue codebase also do not have this line of code.

Steps to reproduce:

1 - Setup a form

import { useForm } from '@inertiajs/vue3'

const form = useForm({ email: '' })

2 - Change form data

form.email = '[email protected]'

3 - Submit form

form.post("/endpoint", {
  onSuccess: () => {
    //after some secconds, make reset
    setTimeout(() => {
      form.reset();
    }, 1000);
  },
});

After this we can see the bug, form.reset will not work, as the values ​​previously sent in the form will become the new default values, breaking the functioning of form.reset

Workaround

As a workaround, it is currently necessary to do a trick

function resetForm() {
    form.defaults({email: ""})
    form.reset()
}

Solution

As a solution, the line that causes this unwanted behavior of resetting default values ​​improperly has been removed.

Copy link
Collaborator

@pedroborges pedroborges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @WillRy! I can confirm that the React and Svelte adapters don't reset the default values after successful submission ✅

@pedroborges pedroborges changed the title useForm wrongly overwrites default values ​​after successful submission [1.x] useForm wrongly overwrites default values ​​after successful submission Sep 13, 2024
Copy link
Contributor

@joetannenbaum joetannenbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

@pedroborges pedroborges merged commit ee1aea7 into inertiajs:master Sep 21, 2024
5 checks passed
pedroborges added a commit to olragon/inertia that referenced this pull request Sep 24, 2024
* master: (95 commits)
  [1.x] Fix props reactivity (inertiajs#1969)
  [1.x] useForm wrongly overwrites default values ​​after successful submission (inertiajs#1935)
  Update changelog
  [1.x] Fix `resetScrollPositions` and `restoreScrollPositions` router methods (inertiajs#1980)
  [1.x] Fix [scroll-region] on html element with overflow-scroll (inertiajs#1782)
  [1.x] Fix useForm re-renders by memoizing functions in React (inertiajs#1607)
  [1.x] Fix "DataCloneError: <Object> could not be cloned" (inertiajs#1967)
  [1.x] preserveScroll should be true on initial page visit (inertiajs#1360)
  Fix type augmentation (inertiajs#1958)
  [1.x] Fix doubling hash in React StrictMode (inertiajs#1728)
  [1.x] Add SSR support for Svelte 5 (inertiajs#1970)
  [1.x] Fix <Render /> component to respect "preserveState" (inertiajs#1943)
  [1.x] Fix 'received an unexpected slot "default"' warning (inertiajs#1941)
  QA: Add @types/lodash to fix svelte-check
  QA: Update reactive if statement
  Review useForm types
  QA: Move the if server up
  QA: Revert tsconfig change
  QA: Remove plural
  QA: Remove unused props type + add extra types just in case
  ...

# Conflicts:
#	packages/react/src/index.ts
reinink added a commit that referenced this pull request Oct 8, 2024
@reinink
Copy link
Member

reinink commented Oct 8, 2024

Hey just a heads up that we decided to revert this change.

I understand the reasoning for the change, but unfortunately whether you want this behavior or the current behavior kind of depends on whether your form is updating something, or if your form is creating something. If you're creating something, then this change makes sense, but if you're updating something the current behavior makes sense. Meaning if we make this change it fixes one situation but breaks another.

Given that, and given that this is technically a breaking change, we've decided to leave this out now and revisit it for v2.0.

I think either way in v2.0 we should get all the adapters working the same way — one way or the other.

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

Successfully merging this pull request may close these issues.

4 participants