Inferno: Selected attribute of option tags are not rendered

[FQ400]

Observed Behaviour

Inferno is omitting selected attribute on <options> when rendering into DOM. Even when selected was set explicitly.

Expected Current Behaviour

Inferno should render the attribute when it is trueish.

Inferno Metadata

macOS, Chrome

Here is the repo with the issue: https://github.com/FQ400/inferno-playground
There is only one component (src/app/components/campaign-type-select). Tests can be run using npm run test. The expected test fails at the moment.

Maybe the issue is related to #773. But I am not sure.

[trueadm]

@FQ400 I'm not entirely sure I get what you mean. selected is a property, so it can be set without it appearing as an attribute on the element in HTML form. Is this issue only for server side rendering or for client side rendering?

[FQ400]

@trueadm The issue is for client side rendering. I was trying to render the selected attribute.

selected is a property, so it can be set without it appearing as an attribute on the element in HTML form

Does it mean it will never appear?

[DoumanAsh]

@trueadm Curious fact is that <option Selected={true}>...</option> produces <option selected="">...</option while selected={true} will not produce it.
Capital seems to matter, but i'm not sure why because it is not documented anywhere

[thysultan]

In the DOM if you grab an element say document.querySelector("option") then check its el.selected property it will reflect whatever inferno has set it to, this may not appear in the developer tools depending on the browsers implementation but the element will have its selected property set to what has been set. <option Selected={true}>...</option> works because inferno doesn't recognised uppercased Selected as a native DOM property so instead of a fast option.selected = true it does option.setAttribute('Selected') which will then display in dev tools and since setAttribute also normalizes case it will also set option.selected to true.

[trueadm]

@FQ400 @DoumanAsh what @thysultan has said is the perfect answer to this. I hope this helps :)

[DoumanAsh]

I think i tested it some time ago and it didn't work the way @thysultan described.
Let me re-check

[trueadm]

@DoumanAsh I tried it just now and it worked for me document.querySelector('option').selected.

[DoumanAsh]

Hm... i'm not sure what i'm doing wrong then but here is example:

class Test extends Component {
    render() {
        return (
            <select>
            <option>1</option>
            <option selected={true}>2</option>
            </select>
        );
    }
}

But i don't get second option selected. Instead i get first option as selected as if Inferno ignored.
If then i hack my way through using capital letter, i'll get second option as selected.

UPD: What is curious a similar attribute disabled works as intended, while there is issue only with selected

[trueadm]

@DoumanAsh Ah, this is indeed a bug. Likely a recent regression too.

@Havunen https://jsfiddle.net/tf4b09nk/

https://github.com/infernojs/inferno/blob/master/packages/inferno/src/DOM/wrappers/SelectWrapper.ts
https://github.com/infernojs/inferno/blob/master/packages/inferno/src/DOM/patching.ts#L772

It appears that because when the value property is omitted (i.e. this is an "uncontrolled component") from <select>, Inferno simply skips applying the selected property as it's on a blacklist. If this was a controlled component this would work.

[DoumanAsh]

I looked over tests and it misses even such test... I think it might be good idea to add test to catch such regression.

[trueadm]

@DoumanAsh can you possibly create a PR that adds such test so we can track this forever from now on – that would be amazing and really appreciated! Once you do, I and the team can prioritize this issue and get a bug fix out. Thank you again!

[DoumanAsh]

@trueadm I created PR to add such test case ( i had issues though to run all tests on Windows, it seems no one is using windows here :) )

[Havunen]

@DoumanAsh what issue did you have? I use windows randomly

[DoumanAsh]

Hm... Actually it seems to be rather strange:

> [email protected] build:packages-cjs D:\repos\inferno
> lerna exec -- ../../node_modules/.bin/tsc --module commonjs --outDir dist --target es5

Lerna v2.0.0-rc.2
'..' is not recognized as an internal or external command,
operable program or batch file.
Errored while executing '../../node_modules/.bin/tsc --module commonjs --outDir dist --target es5' in 'inferno-shared'
Errored while running ExecCommand.execute
Error: spawn ../../node_modules/.bin/tsc ENOENT
    at notFoundError (D:\repos\inferno\node_modules\lerna\node_modules\execa\node_modules\cross-spawn\lib\enoent.js:11:11)
    at verifyENOENT (D:\repos\inferno\node_modules\lerna\node_modules\execa\node_modules\cross-spawn\lib\enoent.js:46:16)
    at ChildProcess.cp.emit (D:\repos\inferno\node_modules\lerna\node_modules\execa\node_modules\cross-spawn\lib\enoent.js:33:19)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:215:12)
'..' is not recognized as an internal or external command,
operable program or batch file.
Waiting for 1 child process to exit. CTRL-C to exit immediately.
(node:8384) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 3): Error: spawn ../../node_modules/.bin/tsc ENOENT
(node:8384) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[Ashot-KR]

@trueadm @Havunen @DoumanAsh i've found something interesting: https://jsfiddle.net/3o5Lq5a8/1/
If anything(text node or HTML-element node) follows <select> element, it renders properly.

This test case will pass

it('should render specified default selected option', () => {
  render(<div>
    <select>
      <option value="a">a</option>
      <option selected={true} value="b">b</option>
    </select>
    <div>:-/</div> {/* <- with this option b will rendered as selected */}
  </div>, container);

  expect(container.querySelector('select').children[ 0 ].selected).to.eql(false);
  expect(container.querySelector('select').children[ 1 ].selected).to.eql(true);
});

[Ashot-KR]

@trueadm @Havunen BTW, shouldn't selected be added in booleanProps?

[Havunen]

if we want to render selected attribute then it should not be boolean prop

[Ashot-KR]

@trueadm @Havunen one more thing after investigation: if select has any props, it also renders with selected second option

<select className="iAmSelect"> {/* <- if select has any prop, option 'b' will be selected */}
    <option value="a">a</option>
    <option selected={true} value="b">b</option>
</select>

---

It looks like algorithm can't reach this point (where processSelect called) because at this point !isNull(props) returns false when there is no any props on select element

[Havunen]

@Ashot-KR all the code you pasted here seems to work just fine?

[Havunen]

Ayea, server side tests didn't work without select being in boolean props. Anyway rendering selected attribute is different thing.

[Havunen]

Released 3.1.0