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

value|defaultValue={Symbol|Function} should be ignored, not stringified #11734

Open
gaearon opened this issue Dec 1, 2017 · 89 comments
Open

Comments

@gaearon
Copy link
Collaborator

gaearon commented Dec 1, 2017

Regression in master from #11534.
Found it thanks to the attribute fixture snapshots.

@nhunzaker
Copy link
Contributor

Just for clarity: This is the correct behavior for Symbols:

screen shot 2017-11-30 at 8 22 43 pm

With my change, value and defaultValue never pass through the property sanitation layer. React tries to stringify symbols, which throws an error, and functions get turned into "function () {}".

@gaearon
Copy link
Collaborator Author

gaearon commented Dec 1, 2017

Now that I looked at this again, the update codepath was already broken. This just made the initial code path match it. So this is actually an improvement. We should still get #11741 to completion to have consistent sane behavior, but IMO this is not as urgent as I thought at first.

gaearon added a commit that referenced this issue Dec 1, 2017
- the capture attribute changed in #11424
- changes to value/defaultValue handling of functions/Symbols are from #11534, but as per #11734 (comment) this is actually not a new problem so we're okay with it
@kanlanc
Copy link

kanlanc commented Jan 6, 2018

Is it resolved or can I start working on it?

Sorry new to open source

@gaearon
Copy link
Collaborator Author

gaearon commented Jan 6, 2018

I think the issue still exists, it’s just more consistent now

@kanlanc
Copy link

kanlanc commented Jan 6, 2018

just for reference how do you call dibs on a bug

@gaearon
Copy link
Collaborator Author

gaearon commented Jan 6, 2018

You got it

@t4deu
Copy link

t4deu commented Feb 1, 2018

Ping @highskillzz, do you still on this issue?

If not, I will take it, ok @gaearon 😉

@kanlanc
Copy link

kanlanc commented Feb 2, 2018

Sure

@raunofreiberg
Copy link
Contributor

raunofreiberg commented Mar 16, 2018

What's the status on this one? It doesn't seem like this has been resolved since there are no warnings on assigning symbols or functions to the defaultValue prop. Even found a few TODO-s in the ReactDOMInput test.

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 9, 2018

Feel free to grab it. I don't know if it's fixed or not but it's worth checking.

@krrishdholakia
Copy link

@gaearon is this still open? if so, how do i claim it?

@philipp-spiess
Copy link
Contributor

philipp-spiess commented Aug 10, 2018

@krrishdholakia Yes this is still up for grabs if it is not already fixed! Feel free to start looking into it and reach out if you have any issues along the way. 😊

@raunofreiberg
Copy link
Contributor

raunofreiberg commented Aug 10, 2018

I checked earlier and I might be wrong but I'm quite sure this was fixed earlier by @nhunzaker

// Same goes for the `value` prop
return (
      <div>
        <input defaultValue={Symbol("test")} />
        <input defaultValue={() => {}} />
      </div>
);

produces

image

Which indicates that both symbols and functions are not stringified? I've still went ahead and added warnings for the defaultValue prop in #13360. Maybe useful.

@raunofreiberg
Copy link
Contributor

Although, textarea seems to behave differently - not sure if intentionally or not.

function App() {
  return (
    <div>
      <textarea defaultValue={() => {}} />
      <textarea value={() => {}} />
      <textarea value={Symbol("test")} />
    </div>
  );
}

Both functions get stringified and appended as text into the textarea itself. The symbol itself crashes the application with a TypeError: Cannot convert a Symbol value to a string

For reproducing: https://codesandbox.io/s/pj40nrp5px

@philipp-spiess
Copy link
Contributor

@raunofreiberg Thanks for looking into that! I agree that textarea should behave the same 👍

@raunofreiberg
Copy link
Contributor

This looks perfect now. Thank you very much 🙂 If you want a follow-up task, you could work on the same fix for elements.

@philipp-spiess Do you mean working on the select tag 🙂? If so, I'd be happy to investigate further on that as well.

@chandanbauri
Copy link

Hi @gaearon ,
Could you please assign issue to me I'm excited to know if this is going to be my first contribution to react .

@kethesainikhil
Copy link

Hi @gaearon,
I am interested to take up this issue and I want work on it could you please assign me this and also I wanted you to Help me to contribute for this I am ready to learn and I am an intermediate coder in react

@koyuncuoglum95
Copy link

Hey @gaearon, I'd love to contribute this project. I'm here for the first time.

@Mule-ME
Copy link

Mule-ME commented Apr 10, 2023

Hey @gaearon, I would love to contribute this project. it's my first time to be here, I am an intermediate coder in react

@its-kunal
Copy link

Hey @gaearon, I am interested in this issue and want to contribute.

@sykalerio
Copy link

Hi there,

My name is [Elpida Syka Lerioti] and I'm interested in helping out with this issue. I've been studying testing and I think this would be a great opportunity for me to apply my skills and learn more about React.

I've read through the issue and I think I have a good understanding of the problem and the proposed solution. However, I'm new to the React project and I would appreciate any guidance or help you could give me to get started with testing.

Thanks for your time and I look forward to working with you!

Best regards,
Elpida Syka Lerioti

@MengLinMaker
Copy link

MengLinMaker commented May 11, 2023

@sykalerio I think this was solved in #22841

A test codesandbox seem to confirm that the stringified behaviour is fixed.

I'm in the same boat, trying to contribute for the first time. I don't know why this issue isn't closed. Maybe I misunderstood the situation.

Can't seem to locate the test file though.

@MengLinMaker
Copy link

@gaearon, should this issue be closed?

@Vishrut19
Copy link

Is this issue still Open?

@bhanu567
Copy link

bhanu567 commented Jul 1, 2023

hey @gaearon is this issue still open ??

@theQuery
Copy link

theQuery commented Jul 4, 2023

Should this be closed or is there still some unexpected behavior occurring with this? 🤔

@NIKHILNAIR21
Copy link

is this issue is still open? @gaearon

@Tabhi109
Copy link

@gaearon Is this issue still open , Please assign it to me

@ValeriaHoody
Copy link

Hey, is this issue still open, interested to work on it.

@SouSingh
Copy link
Contributor

Is it still open or closed? I would like to work on it

@DerickT02
Copy link

Is this still open?

@IshadP
Copy link

IshadP commented Sep 17, 2023

Is the issue still open? I am interested to work on it

@shobhnaawasthi
Copy link

Does this issue still persist? I am interested to work on it

@AmKreta
Copy link

AmKreta commented Oct 23, 2023

@gaearon is this still open ? I have found a solution and want to make a pr !

@yogesh10-r
Copy link

Hi @gaearon !
is this still open? I'd like to contribute...

@mishchief
Copy link

For everyone coming here as per #11734 (comment)

This issue has been fixed...

@sankshit
Copy link

sankshit commented Feb 1, 2024

@gaearon It would be helpful if you guys can close this issue

@ScriptShah
Copy link

Hello , I'm here for the first time and would like to contribute this project.

@chiragmetaliya
Copy link

Hello, I believe I could contribute to this project. Can someone please clarify whether the issue is closed? I'm here for first time and it seems from previous thread comments that this issue is already closed.

@mishchief
Copy link

Hello, I believe I could contribute to this project. Can someone please clarify whether the issue is closed? I'm here for first time and it seems from previous thread comments that this issue is already closed.

Read the thread this is not an issue anymore and needs to be closed...

@AliSinaYOusofi
Copy link

how come this is not closed yet !

@Tayyabbutt1133
Copy link

Tayyabbutt1133 commented Jul 25, 2024

I think the issue still exists, it’s just more consistent now

@gaearon is the bug is still there ? If yes, can I work on this ?

@WajahatKanju
Copy link

@gaearon if the bug is not there please close the issue. Other I am interested in working on it.

@frozenfrank
Copy link

I think it's hilarious that this:

  1. Was created 7 years ago
  2. is one of only two issues marked as "good first issues,"
  3. is already resolved,
  4. attracts a lot of attention from people requesting to have the issue assigned to them (but no one seems interested in just proposing a PR), AND
  5. is still open.

This shows poor repo management.

image

@rahul3002
Copy link

Hi! I'm interested in working on this task. It seems straightforward, so I’ll give it a try and keep you updated on my progress. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests