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

[idea] React-enhanced input/textarea/select should be addons #2255

Closed
wants to merge 2 commits into from
Closed

[idea] React-enhanced input/textarea/select should be addons #2255

wants to merge 2 commits into from

Conversation

syranide
Copy link
Contributor

EDIT: I recommend clicking this link to view changes as github diff ignored the file moves.

This RFC-PR removes all React-specific enhancements from ReactDOMInput, ReactDOMTextarea and ReactDOMSelect. The controlled (and still optionally uncontrolled) implementations are now found in React.addons.* instead and are fully backwards-compatible. The implementations provided by React core are now bare-bones, focusing only on exposing the DOM in a predictable way, much like the rest of ReactDOM.

Related: #2242, #2241, #2220, #1968, the list goes on.

Why was it flawed

ReactDOMInput, ReactDOMTextarea and ReactDOMSelect are very complex implementations and they're still far from perfect (and quite frequently patched). Worse, the implementations are really rather hacky. A generic neat and robust implementation simply doesn't seem possible. <select> has to refresh option.selected for all options every render, the logic for <input type="radio"> is down-right horrendous and there are a number of edge-cases that do not behave as you would expect.

Trying to make <input>, <textarea> and <select> controlled for the generic use-case is flawed and as such doesn't belong in React core.

Why is this better

ReactDOM* is mostly wrapped by users and packaged as their own MyTextfield, MyButton, etc components and React makes it fantastically simple. So there's no reason why they should be forced to use and bundle sub-optimal implementations even when they have significantly simpler requirements.

Implementing the React-enhanced controlled versions of <input>, <textarea> and <select> as addons/components is trivial. It allows the core implementations to stay lean and focus only on exposing the DOM consistently (and to patch ReactDOM* independently of React's release schedule). It allows the community to tailor the amount of logic to their own needs and to ignore edge-cases that simply aren't relevant, if they don't care, just use React.addons.*. Providing "pure" implementations of the controls would be trivial too for even more benefit.

There's no reason why this should be in React core. This PR as-is shaves ~4.3kb/1.2kb (~3-4%) of React core, can significantly reduce performance overhead for edge-cases and keeps React ever slightly less needlessly opinionated.

Result

ReactDOM* now only accept initial values. <input initialValue={...} initialChecked={...} />, <textarea initialValue={...} /> and <select initialValue={...} /> (initialSelected?), feel free to object to initial vs default.

React.addons.* are backwards-compatible implementations building on-top of the ReactDOM* exposed by React core.

Codemod:ing existing codebases should be trivial.


PS. This PR is not thoroughly coded/tested (yet) and includes some unrelated fixes (and obviously breaks a lot of tests), but the idea is sound and the implementation does work. If you agree with the premise I could go ahead and clean it up.

cc @zpao @petehunt @sebmarkbage @jordwalke @spicyj @chenglou

@syranide syranide changed the title RFC: React-enhanced input/textarea/select should be addons [idea] React-enhanced input/textarea/select should be addons Sep 29, 2014
@sebmarkbage
Copy link
Collaborator

Marking as needs revision because this will need an upgrade path through an intermediate release. Let's wait on that while we settle the conversation if we're going to keep these or not.

@sophiebits
Copy link
Collaborator

I am not a fan of this. Exposing the raw DOM components seems reasonable if it allows people to build better abstractions, but people won't use controlled components if they aren't in the main build and (unless I'm missing something) that's how we recommend people to build things.

@jimfb
Copy link
Contributor

jimfb commented Nov 10, 2015

Merge conflicts, and no discussion for over a year. It looks like we've settled on keeping controlled components within the core. Closing this out.

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

Successfully merging this pull request may close these issues.

5 participants