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

<input value={undefined}> shows string "undefined" #1041

Open
edemaine opened this issue Jun 6, 2022 · 5 comments
Open

<input value={undefined}> shows string "undefined" #1041

edemaine opened this issue Jun 6, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@edemaine
Copy link
Contributor

edemaine commented Jun 6, 2022

Describe the bug

This is a sequel to #955, where the same issue occurred with class/className.

<input value={undefined}/>

compiles to

const _el$ = _tmpl$.cloneNode(true);
_el$.value = undefined;
return _el$;

Because of the infinite wisdom of the DOM .value interface, this causes the value to be set to the string "undefined". 🙁

Reported on Discord

Your Example Website or App

https://playground.solidjs.com/?hash=-335339517&version=1.4.1

Steps to Reproduce the Bug or Issue

  1. Go to Playground link
  2. Observe input value.

Expected behavior

I'm not sure there's a clear expectation here, but I probably expected a blank input, with the intuition that attr={undefined} is equivalent to omitting attr altogether. Instead I'm seeing the string "undefined" in the input.

Screenshots or Videos

image

Platform

  • OS: Windows
  • Browser: Chrome
  • Version: 102

Additional context

FWIW, here's a CodeSandbox showing that value={undefined} renders blank in React.

Is it worth reviewing for other DOM interfaces like these?

@ryansolid
Copy link
Member

It's only going to be the short list of properties I choose to handle as properties for performance. I just double-checked and it's value and className and boolean attributes. Unfortunately adding checks both increases bundles size and worsens performance. Although not as much as using attributes I suppose.

I think if there is a good reason for a value helper probably should consider it. What finally pushed className was the desire to actually remove the attribute. I think people come across that one more because they write conditional logic. With value you probably only hit this if you are initializing and not using TypeScript.

The solution is to initialize to an empty string. It is so much better if we can just not do anything here as it is so easy to mitigate and the solution puts a size and perf tax on everyone. But if there was more reason to go this way definitely do it.

@edemaine
Copy link
Contributor Author

edemaine commented Jun 6, 2022

Agreed; I expect a type error when setting value to undefined, so it seems fine to leave this behavior, provided we document and perhaps help avoid it.

  1. In TypeScript, I don't actually see a type error here. I guess this requires exactOptionalPropertyTypes which was brought up recently. If we can properly distinguish between "optional" (?) and "undefined allowed" (| undefined) we could recommend people turn on this option.

  2. In dev mode, we could detect this and issue a warning. Though it's pretty obvious what's going on here when you see it, so I'm not sure this is necessary.

@martinpengellyphillips
Copy link

I just encountered this after updating an older project. Curious when this behaviour changed as it didn't seem to happen before?

@ryansolid
Copy link
Member

Always been like this. This is an example where we'd need to do more work to prevent it.

@mdynnl
Copy link
Contributor

mdynnl commented May 12, 2024

seems to be done in ryansolid/dom-expressions@e69bf27 (class) and ryansolid/dom-expressions@4aca0d2 (attributes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants