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

Umbrella 0.13 #2809

Closed
14 of 23 tasks
sebmarkbage opened this issue Jan 6, 2015 · 9 comments
Closed
14 of 23 tasks

Umbrella 0.13 #2809

sebmarkbage opened this issue Jan 6, 2015 · 9 comments

Comments

@sebmarkbage
Copy link
Collaborator

Won't fix:

  • Expose DOM component refs as the actual DOM nodes. (Monkey patch props, setProps, getDOMNode onto it as an upgrade path.)
  • Warnings on using a DOM Component's ref's getDOMNode or props
  • Fix the error utils guard. See ReactCompositeComponentError-test
  • Deprecate setProps? What's the alternative?
  • expose cloneWithProps on React proper (not actual cloneWithProps but the other one)
  • Deprecate replaceState?
@sophiebits
Copy link
Collaborator

Deprecate keyed objects as children sometime?

@sebmarkbage
Copy link
Collaborator Author

ah yes.

@syranide
Copy link
Contributor

@sebmarkbage getDOMNode() is not defined for React.Component, shouldn't it fall under "warn for one release and drop the next"? PS. Well... it is defined with a warning, but it doesn't actually return a function.

@sebmarkbage
Copy link
Collaborator Author

@syranide No, because React.Component didn't exist in previously releases. Not exposing it explicitly to prevent new code from relying on it.

@syranide
Copy link
Contributor

@sebmarkbage Makes sense, the problem I see with it is this.refs.foobar.getDOMNode() will now fail "arbitrarily", depending on whether the component moved away from React.createClass or not. I realize these are fairly rare circumstances, but it did bite me. I.e, it is not only the component itself that is affected, but the consumer of the component as well.

@sebmarkbage
Copy link
Collaborator Author

@syranide That's why we also have React.findDOMNode(this.refs.foobar) which moves the responsibility out of the instance itself into a global escape hatch.

@syranide
Copy link
Contributor

@sebmarkbage Yeah definitely, what I'm saying is just this breaks the "first warn then break" approach React seems to generally take. Now, doing npm update and also getting an updated component could mean your app instantly breaks telling you to use React.findDOMNode rather than warning and also working as it should for now. But I personally don't mind, I've already fixed all call-sites... just saying :)

@sebmarkbage
Copy link
Collaborator Author

This scenario doesn't break because of npm update but because you're also adding a new component which is why I think it is fine. :)

@andreypopp
Copy link
Contributor

@syranide those who maintain React components and upgrade from React.createClass(...) to ES6 classes should bump major version then. Probably a good thing to be noted in 0.13 release announcement.

@zpao zpao closed this as completed Feb 21, 2015
@sebmarkbage sebmarkbage mentioned this issue Feb 21, 2015
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants