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

Select becomes blank when value is provided #139

Closed
iluuu1994 opened this issue Sep 16, 2021 · 5 comments
Closed

Select becomes blank when value is provided #139

iluuu1994 opened this issue Sep 16, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@iluuu1994
Copy link

Thank you for this library!

Summary

Consider this example:

<script>
    import { createForm } from 'svelte-forms-lib';

    const { form, handleSubmit, handleChange } = createForm({
        initialValues: { a: 1 },
        onSubmit: values => {
            alert(JSON.stringify(values));
        },
    });
</script>

<form on:submit={handleSubmit}>
    <select name="a" value={$form.a} on:blur={handleChange}>
        <option />
        <option value={0}>Foo</option>
        <option value={1}>Bar</option>
        <option value={2}>Baz</option>
    </select>
    <button type="submit">submit</button>
</form>

When selecting a different elementin the dropdown the select field becomes blank. Interestingly the element is still selected in the store because when the form is submitted the right value is returned.

svelte-forms-lib

Example Project

Of course, it works fine on codesandbox for some reason 😒 I don't understand why.

https://codesandbox.io/s/svelte-forms-lib-select-empty-8ctpk?file=/App.svelte

So I've created a small sample repository.

https://github.com/iluuu1994/svelte-forms-lib-select-error

What is the current bug behavior?

The selected element becomes blank.

What is the expected correct behavior?

The selected element should remain the one I've selected.

@iluuu1994 iluuu1994 added the bug Something isn't working label Sep 16, 2021
@iluuu1994
Copy link
Author

Oh, it turns out the values are the problem. Making them strings works. I used numbers because the values are coming from a database.

@iluuu1994
Copy link
Author

I looked at this more closely. The problem is that this library uses the DOM to get the given option-value. Unfortunately getting the value from the DOM means the value looses it's original type and is coerced to a string.

const value = isCheckbox(element) ? element.checked : element.value;

Svelte stores the original value of the option in a property called __value. It would be possible to get the value this way, although I'm not sure if __value is guaranteed to stay there or is more of an implementation detail.

https://svelte.dev/repl/626573ae7c6d47398aa3ee1173ae746b?version=3.43.0

function handleChange(e) {
	const target = e.target;
	
	let value;
	if (target.multiple) {
		value = Array.from(target.querySelectorAll('option:checked')).map(o => o.__value);	
	} else {
		value = target.querySelector('option:checked')?.__value;
	}
	
	console.log(value);
}

Would you be ok with this change? If so, I can create a PR.

@larrybotha
Copy link
Collaborator

@iluuu1994 thanks for your feedback!

Ye, I've experienced this, too. I was surprised at first, but this appears to be the way the DOM works, so I worked around the issue with strings, too 🤷

IIRC, values in DOM inputs are always strings, even input[type=number] fields seem to hold values as strings when retrieved via input.value. I probably found this surprising because of my experience with React, where we're not actually working with the DOM, and types are preserved.

__value is likely a Svelte internal, and we don't want to go down that road, as it'll introduce maintenance overhead and potentially a surprising breaking change if the upstream changes.

Feel free to give this a bash if you'd like! I'm inclined to augment the DOM as little as I can and treat Svelte as a light abstraction on top of what the DOM already does, but there's potentially an elegant solution here!

@iluuu1994
Copy link
Author

Do you have a specific idea? For the Select component there's a simple solution: Bind to a local variable and reactively call updateValidateField. Unfortunately I don't know how this library could solve this without bind:value where the user would not use Select but implement it themselves.

@larrybotha
Copy link
Collaborator

larrybotha commented Nov 6, 2021

@iluuu1994 on thinking more about this, a decorator on a per-element basis seems to be the good way to handle this. It may be a little more work for users (although a decorator would only need to be written once), doesn't add any complexity to svelte-forms-lib.

// untested
<script>
	const {handleChange, ...} = createForm(...)
	
	function handleSelectChange(event) {
		const {value: inputValue} = event.currentTarget
		const maybeInt = Number.parseInt(inputValue, 10)
		const value = !Number.isNaN(maybeInt)
			? maybeInt
			: inputValue
		
		// mutate the event object directly 
		Object.assign(event, {target: {value}})

		handleChange(event)
	}
</script>

<form ...>
	<select on:change={handleSelectChange}>
		<option value="123">my option</option>
	</select>
</form>

I'm gonna close this out, as strings are the expected type for HTMLSelectElement, but feel free to discuss further :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants