alpinejs / alpine

A rugged, minimal framework for composing JavaScript behavior in your markup.
https://alpinejs.dev
MIT License
27.92k stars 1.22k forks source link

Fix form reset for x-model radio, checkbox arrays, select multiple and various modifiers #4159

Closed bb closed 4 months ago

bb commented 4 months ago

This PR fixes form reset for many input types (radio, checkbox array, select multiple) plain as well as in combination with number and boolean modifiers for casting as well as fill modifier for <select multiple and also fixes duplicated entries with checkbox arrays during reset.

A few tests have been added, all of them were failing before this PR and are good now.

Some of the fixes in more detail:

... and possibly a few more.

bb commented 4 months ago

I started out adding more and more conditions in the reset listener but settled on simply always using the getInputValue approach because I basically ended up duplicating all the conditions from there anyway. In between it looked like this:

nextTick(() => el._x_model && (el.type !== 'radio' || el.checked) && (((el.type === 'checkbox' && Array.isArray(getValue())) || el.type === 'radio' || (el.tagName.toLowerCase() === 'select')) ? setValue(getInputValue(el, modifiers, { target: el }, getValue())) : el._x_model.set(el.value)))

now it's just

nextTick(() => el._x_model && el._x_model.set(getInputValue(el, modifiers, { target: el }, getValue())))

I kept using the public setter (which was prepared to allow programmatic overriding of x-model as described in a comment below that line). However, there are some places in the code which do not use the overridable setter: cloning and when using .fill. So I wonder if there are more bugs hidden in this area. I recommend adding a test for overriding the setter to make sure there are none.

calebporzio commented 4 months ago

Thanks @bb! Thanks for all your work on this