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

Investigate why select._wrapperState.initialValue is necessary #13410

Closed
nhunzaker opened this issue Aug 16, 2018 · 7 comments
Closed

Investigate why select._wrapperState.initialValue is necessary #13410

nhunzaker opened this issue Aug 16, 2018 · 7 comments

Comments

@nhunzaker
Copy link
Contributor

nhunzaker commented Aug 16, 2018

Uncovered during @raunofreiberg's select work (#13389).

select._wrapperState.initialValue is only ever assigned, never referenced. Can it be removed?

Example:
https://github.com/facebook/react/blob/master/packages/react-dom/src/client/ReactDOMFiberSelect.js#L191

It would be great if someone could investigate this, and figure out if it can be removed.

@nhunzaker
Copy link
Contributor Author

nhunzaker commented Aug 16, 2018

Nominating @raunofreiberg for this, but I don't want to create work for them :)

@jquense
Copy link
Contributor

jquense commented Aug 16, 2018

I want to say that this is an artifact of older defaultValue handling for selects?

@nhunzaker
Copy link
Contributor Author

Removing it completely appears to have no effect on the test suite.

@kartiklad
Copy link
Contributor

@nhunzaker Can I take this one? This is my first time contributing to React

Purely from reading the code, it seems like your assessment of initialValue not being used seems correct. Happy to investigate further

@nhunzaker
Copy link
Contributor Author

Go for it!

@kartiklad
Copy link
Contributor

Hey @nhunzaker I raised a PR for the change #13412

@nhunzaker
Copy link
Contributor Author

Fixed by #13412. Thanks @kartiklad!

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

3 participants