Bug: Select when passed a value as Prop errors with a suggestion to pass readOnly

[Biki-das]

Seems weird when we try to use a select component, and pass a value as a prop, it prompts with the error to either set onChange or readOnly.
the readOnly at the last seems misleading since the select component does not have a readOnly prop, unlike inputs which do have a readOnly attribute.

SELECT PROPS
https://react.dev/reference/react-dom/components/select

seems an easy fix would be to check if the mounted component is a select component and then conditionally just change the error message, though would need to add tests to support the same.

Screen.Recording.2023-11-06.at.9.27.49.AM.mov

cc @sophiebits

codesandbox link to tinker:- https://codesandbox.io/s/admiring-wright-9sctsn?file=/src/App.js

[Biki-das]

I can work on this and make the error message correctly prompt.

[felipe-rodolfo]

export default function App() {
  const [selectedValue, setSelectedValue] = useState("fruits");

  return (
    <div className="App">
      <select value={selectedValue} onChange={(e) => setSelectedValue(e.target.value)}>
        <option value="Mango">Mango</option>
        <option value="Apple">Apple</option>
        <option value="Pears">Pears</option>
      </select>
      <input value="hello" readOnly={true} />
    </div>
  );
}

Does this work in your case?

[Biki-das]

export default function App() {
  const [selectedValue, setSelectedValue] = useState("fruits");

  return (
    <div className="App">
      <select value={selectedValue} onChange={(e) => setSelectedValue(e.target.value)}>
        <option value="Mango">Mango</option>
        <option value="Apple">Apple</option>
        <option value="Pears">Pears</option>
      </select>
      <input value="hello" readOnly={true} />
    </div>
  );
}

Does this work in your case?

@felipe-rodolfo the bug is not related to how i should use the Select component, the error message is the Problem since it says to pass a readOnly value to pass a prop as the option for the Select component which does not has the same and does not seem to be a good developer experience.

[Biki-das]

cc @hoxyq do lmk if this something valuable to fix!!

[hoxyq]

cc @hoxyq do lmk if this something valuable to fix!!

I think it is worth fixing, especially given the fact that https://react.dev/reference/react-dom/components/select doesn't list readOnly as an attribute.

seems an easy fix would be to check if the mounted component is a select component and then conditionally just change the error message, though would need to add tests to support the same.

I am not familiar with the code where this happens, but maybe its worth just implementing its own validation for select component.

[Biki-das]

ok will look up we have holidays here till next week.
seems an easy fix would need to inspect the component and filter the message and add tests to support the same :-)

[Biki-das]

@hoxyq can you change the status to confirmed if possible?

[Biki-das]

@hoxyq could you spare a few minutes to run the fixtures/packaging/babels-standalone/dev.html file ? for some reason ReactDOM.render() is not defined and on console.log(ReactDOM) it returns an empty Object.
am i doing something wrong?

if you are able to see hello world rendered could you just share the snippet and i can then test my changes.

[hoxyq]

@hoxyq could you spare a few minutes to run the fixtures/packaging/babels-standalone/dev.html file ? for some reason ReactDOM.render() is not defined and on console.log(ReactDOM) it returns an empty Object. am i doing something wrong?

if you are able to see hello world rendered could you just share the snippet and i can then test my changes.

Can confirm that it doesn't work for me also. Looks like a recent regression, but I don't have the time now to bisect the PR, which introduced this. I've validated that sourcemaps PRs are not related to it, so there is some commit prior to it that breaks this.

[Biki-das]

ok @hoxyq i understand appreciate your investigation, any other way i can test my changes if you know, i tried copying the node modules from the oss/experimental and putting the code of both react and react-dom inside the dummy react project of mine, but it is also not picking any changes.

[hoxyq]

ok @hoxyq i understand appreciate your investigation, any other way i can test my changes if you know, i tried copying the node modules from the oss/experimental and putting the code of both react and react-dom inside the dummy react project of mine, but it is also not picking any changes.

Jest tests, probably? Or try checking out on some older commit. I've tried main~100 and it works.

[Biki-das]

ok @hoxyq i understand appreciate your investigation, any other way i can test my changes if you know, i tried copying the node modules from the oss/experimental and putting the code of both react and react-dom inside the dummy react project of mine, but it is also not picking any changes.

Jest tests, probably? Or try checking out on some older commit. I've tried main~100 and it works.

Thanks for the follow up, will try doing the same

[hoxyq]

@hoxyq could you spare a few minutes to run the fixtures/packaging/babels-standalone/dev.html file ? for some reason ReactDOM.render() is not defined and on console.log(ReactDOM) it returns an empty Object. am i doing something wrong?

if you are able to see hello world rendered could you just share the snippet and i can then test my changes.

Should be fixed with #27717.

[Biki-das]

@hoxyq could you spare a few minutes to run the fixtures/packaging/babels-standalone/dev.html file ? for some reason ReactDOM.render() is not defined and on console.log(ReactDOM) it returns an empty Object. am i doing something wrong?
if you are able to see hello world rendered could you just share the snippet and i can then test my changes.

Should be fixed with #27717.

thanks @hoxyq , i have also found the file to change and return the appropriate error, once this is merged, i will take a pull and test the changes locally and add appropriate tests.

[vincerubinetti]

Just ran into this and was very confused.

I think it is worth fixing, especially given the fact that https://react.dev/reference/react-dom/components/select doesn't list readOnly as an attribute.

To back this up, the TypeScript definitions also do not accept readOnly on a <select>, and MDN says it's not allowed (so I guess it's in the spec, though this doesn't make sense to me).

---

Here's the thing about just changing the error message though...

I'm using Zag.js for a single/multi-select component, and I have the following code to render a hidden input, which exists solely to provide a value to FormData:

<select
  {...api.hiddenSelectProps}
  value={multi ? api.value : api.value[0]}
>
  {options.map((option, index) => (
    <option key={index} value={option.id}>
      {option.text}
    </option>
  ))}
</select>

Zag already tracks the state via a statemachine, and the user is able to control the input via other controls that I didn't show. Therefore I really need a value and don't need an onChange. Adding disabled makes the error go away, but also omits the field from FormData.

So as far as I can see, to make the error go away, I have to provide onChange={() => null}. This feels a bit strange and dirty, so I think it's worth considering whether the rules you're checking here are truly air-tight. I think they usually are, and this is just a really weird edge case...

[hoxyq]

Just ran into this and was very confused.

I think it is worth fixing, especially given the fact that https://react.dev/reference/react-dom/components/select doesn't list readOnly as an attribute.

To back this up, the TypeScript definitions also do not accept readOnly on a <select>, and MDN says it's not allowed (so I guess it's in the spec, though this doesn't make sense to me).

Here's the thing about just changing the error message though...

I'm using Zag.js for a single/multi-select component, and I have the following code to render a hidden input, which exists solely to provide a value to FormData:

<select
  {...api.hiddenSelectProps}
  value={multi ? api.value : api.value[0]}
>
  {options.map((option, index) => (
    <option key={index} value={option.id}>
      {option.text}
    </option>
  ))}
</select>

Zag already tracks the state via a statemachine, and the user is able to control the input via other controls that I didn't show. Therefore I really need a value and don't need an onChange. Adding disabled makes the error go away, but also omits the field from FormData.

So as far as I can see, to make the error go away, I have to provide onChange={() => null}. This feels a bit strange and dirty, so I think it's worth considering whether the rules you're checking here are truly air-tight. I think they usually are, and this is just a really weird edge case...

IMHO, setting onChange to no-op is explicit and looks aligned with current react-dom APIs . If react-dom's select had hidden attribute, we could've also took it into account before printing an error.