Umbrella 0.14 #3220

Closed
sebmarkbage opened this Issue Feb 21, 2015 · 37 comments

Projects

None yet
@sebmarkbage
Member

New Cool Features

Package changes

  • Build Separate Packages for react isomorphic, dom and server rendering [Still need to handle https://github.com/facebook/react/issues/4279 ] (@zpao)
  • Deprecate the dom renderer in the react package. Replace with separate package. Warnings if used. (@zpao)
  • Ship all React addons as separate packages. Remove from react repo. (Possibly kill already deprecated ones like classSet) Build with warnings if existing package is used? (@zpao)
  • End-of-life of react-tools. Clean up legacy in react-native and in the react repo (@zpao)
  • Ship index.ios.js with react so that react-native components can immediately start using the react package. (@sebmarkbage)
  • Decouple dependencies between React DOM and React Isomorphic, except for currentOwner which React DOM still needs to set on React Isomorphic. (@sebmarkbage) See comment in #3866
  • Browser build for ReactDOM (@zpao)

Kill Deprecations from 0.13

  • Object.freeze props in createElement in DEV, remove old props mutation warnings. (@jimfb) see #4172.
  • Make React.addons.createFragment return a toArray from objects (@spicyj)
  • Kill _isReactElement (@spicyj)
  • Kill ComponentClass.type (@jimfb) see #4009
  • Clean up the remaining owner/context stuff (@jimfb)

New Deprecations

  • Deprecate setProps/replaceProps and replace with external ReactRenderer package, don't forget to include replaceProps in the renderer API and ship the package. (@jimfb, see #4150)
  • Deprecate getDOMNode (replaced with React.findDOMNode) (@spicyj)
  • Expose DOM component refs as the actual DOM nodes. #3223 (@spicyj)
  • Warnings on using a DOM Component's ref's getDOMNode or props #3223 (@spicyj)
  • Clean up remainders of context (@jimfb)

Probably not

  • Deprecate replaceState? #3236 Needs Immutable.js solution. (@sebmarkbage)
  • Deprecate isMounted? #3417 Needs Promise solution. (@sebmarkbage)
  • Detect event target through event.path if available. E.g. in shadow DOM. (@jimfb) see #4150. (Disabled in #4585)
@syranide
Contributor

Expose DOM component refs as the actual DOM nodes. (Monkey patch props, setProps, getDOMNode onto it as an upgrade path.)

Hmm, this seems kind of weird to me, especially in-light of ReactDOMInput, etc unless we intend to discontinue them as (recommended) addons or w/e. I would think the preferable direction would be away from direct DOM interaction. Going off on a tangent here; including in some sense basically discontinuing the use of DOM events in-favor of passing a ReactEvent or even just the new value to onChange callbacks (which perhaps only really makes sense as an addon perhaps).

@sebmarkbage
Member

Breaking this out into a separate discussion in #3223. Not sure what you mean by "passing a ReactEvent"...? Anyway we

@kharin
kharin commented Feb 22, 2015

Deprecate replaceState? What about things like: https://github.com/facebook/immutable-js/wiki/Immutable-as-React-state

@sebmarkbage
Member

@kharin As that page mentions, it is safer to use a nested state property anyway.

We could build native support so that this would work:

  getInitialState() {
    return Immutable.Record({count:0})();
  },
  handleClick() {
    this.setState(this.state.update('count', v => v + 1));
  },

We would merge it in React. We would likely only support the Record type, and not Maps though. It would still have problems with mixins.

@kharin
kharin commented Feb 23, 2015

@sebmarkbage the Record trick looks great! I'm not sure what would be performance implications, though. If React were to merge it as usual it's likely to be slower since V8 de-ops property setters.

Just wanted to know you thoughts on the practice. So, if I understood correctly, using replaceState() (in this use-case) is discouraged until React lends some sort of native support?

@sebmarkbage
Member

That's right. I added a spin-off Issue for discussion. #3236

@busticated

I wouldn't be surprised if i'm just coming to this conversation without the proper context, etc (still somewhat new to react :) but perhaps this will be helpful to others in my position. I have two basic questions:

  1. re: #3223 - today i use refs to get a handle on a component - sometimes to call .getDOMNode(), sometimes just to call some method on the instance itself. while i realize this might not be the ideal, it's super-handy. is the plan to prevent this kind of thing or will it continue to be supported in a different way?

  2. re: Deprecate setProps - i currently have a top-level app component which receives all of its data / state via .setProps() & .setState(). for this use-case, is the aim to get folks to move over to using .render() directly for prop updates? (i realize the docs suggest this approach) will we still have access to the component instance / .setState() ?

thanks for a great lib :)

@spicyj
Member
spicyj commented Feb 27, 2015
  1. Yeah, refs to components you define yourself will still work the same way – #3223 talks just about what happens if you take a ref to a node directly, like <img ref="photo" />. We're looking at making this.refs.photo be the img node directly in that case.

  2. Yeah, re-calling React.render at the top level is the preferred solution. No plans to change how the instance is returned, though you should treat state as private to a component and only call setState on yourself (i.e., this.setState) and never on another instance you have a handle to. You can expose a custom setter method, but in most cases it's better to just pass the new values via props.

@busticated

@spicyj great - thanks!

@staltz
staltz commented May 30, 2015

Stateless Functions https://github.com/reactjs/react-future/blob/master/01%20-%20Core/03%20-%20Stateless%20Functions.js (who wants to do it? should be easyish.)

Naming nitpicking: "Stateless function" is redundant. It's like saying "Wingless dog". "Functions as stateless components" is more correct.

@spicyj
Member
spicyj commented May 31, 2015

Well, React supports winged birds, wingless birds, and (soon) wingless dogs. I'll change the name in the checklist above though.

@sebmarkbage
Member

Every science and community has contextual terminology where you drop redundant terms. All these terms are also contextual to our field. I'm sure they mean something completely different in the world of plumbing and they would prefer it if we add more context.

In the world of the React community, the word "component" is often redundant. For example, when we say "instance" we often drop the "component" part.

In the broader sense, a "stateless function" is NOT redundant since it clarify the difference from a "closure" with captured state. You could build one of these single function components by using the closure to capture state for callbacks. In fact, that has been suggested.

@staltz
staltz commented Jun 1, 2015

The presence or absence of a function closure is orthogonal to statefulness. A function can have references to constants in a closure and still be a pure function. Or a function might have no closure attached, and still be side-effectful, by taking as parameter a reference to some mutable data structure and mutating it. That's the Deku approach actually.

React isn't really that separated from the rest of the programming community, and "Science" for that matter. You're using well known concepts such as nodes on a UI tree, and pure functions. Why not name it correctly instead of risking confusing programmers by introducing creative names for pretty common programming concepts.

@sebmarkbage
Member

"closure" with captured state

function closure is orthogonal to statefulness

That's why I clarified that it captured state. I don't need a condescending terminology lesson, thanks.

This is an internal React issue tracker, used for our communication and tracking of issues, and not a blog post broadcasting new creative concepts. It is expected that the reader is aware and understands the distinction.

@sebmarkbage
Member

I should clarify why this matters.

Doing development in the open is a curtesy. Lots of teams do this kind of progression in private. We even do that way too much by virtue of face-to-face meetings. I would like us to be more transparent. To make that happen, we don't want to be in a situation where we have to tailor our communication in a clear and timely fashion. That's what blog posts are for.

I definitely don't want anyone to have to hesitate to post because they feel they'll need to have a clear chain of arguments, make it understandable to a broader audience without context or because of fear from the terminology police. This space should be as accessible to our core team as possible. Even if that means contextual terminology, missing information if you're not part of wider conversations or even exploratory concepts that may or may not end up at already commonly known concepts.

That's why I'm pushing back. Feel free to ask clarifying questions though.

@staltz
staltz commented Jun 1, 2015

It was just a side note.

@spicyj spicyj added this to the 0.14 milestone Jun 2, 2015
@spicyj
Member
spicyj commented Jun 2, 2015

@jimfb Can I pass "Object.freeze props in createElement in DEV, remove old props mutation warnings." to you since you're handling the deprecation internally?

@jimfb
Contributor
jimfb commented Jun 2, 2015

Yes, I'll update the umbrella issue.

@chicoxyzzy
Contributor

Deprecate getDOMNode (replaced with React.findDOMNode)

it's already deprecated, isn't it? or should this be in Kill Deprecations from 0.13 group?

@spicyj
Member
spicyj commented Jun 5, 2015

@chicoxyzzy 0.13 didn't have a runtime warning for it.

@nmn
Contributor
nmn commented Jun 25, 2015

@spicyj maybe release a point release now and add some deprecations. So that when 0.14 comes around you can kill more code. There seems to be some work and time left for 0.14 release.

@jimfb
Contributor
jimfb commented Jun 25, 2015

@nmn We generally don't add any new functionality/deprecations to point releases (only minor bug fixes). Also, we're working hard to get 0.14 out the door, and increasing the scope of the release won't make it happen any faster :).

@nbostrom
Contributor
nbostrom commented Jul 7, 2015

Does the fact that you released the beta without #3995 mean that you are dropping it from 0.14?

@jimfb
Contributor
jimfb commented Jul 7, 2015

@nbostrom No promises, but I think we're still going to try to squeeze that in for 0.14. We were just a little too busy before ReactConf Europe, and it wasn't quite ready in time for beta1.

@nbostrom
Contributor
nbostrom commented Jul 7, 2015

Thanks for the quick answer. I do hope you find the time, but at the end of the day I guess that it's really just a nice-to-have.

@sebmarkbage
Member

#3995 hit a block because some unfortunate timing in the initialization code forcing an unfortunate branch. I think we can probably fix that and get a better implementation by restructuring some other code.

Priorities in terms of releases sometimes get prioritized based on what gets blocked on it. I.e. if we don't add/deprecate certain things in release A then we can't do something else in release B. So this kind of sequencing is important. New APIs like this one often doesn't block anything else so they're easy to bump. They can always go into an ad-hoc release later on though. So we should be able to get it in soon.

@EvNaverniouk

I'm currently experimenting with migrating to 0.14 beta, and having a lot of trouble with the deprecation of setProps due to the fact that our app makes a lot of use of portal components (ie. for modals).

I'd like to request that the official 0.14 release include some extra docs/examples of how to migrate away from setProps. The current error message (Warning: setProps(...) and replaceProps(...) are deprecated. Instead, call React.render again at the top level.) is not sufficient as it's not immediately clear how to call React.render on a component that's already been mounted (especially if using renderSubtreeIntoContainer).

EDIT Feel free to message me on Slack @ reactiflux for additional info. My username is globex

@spicyj
Member
spicyj commented Jul 14, 2015

Just re-call React.render (or renderSubtreeIntoContainer) with the same container and your component will update. This has always been our recommended way to update a top-level component.

@jimfb
Contributor
jimfb commented Jul 14, 2015

@spicyj No, that doesn't solve the setProps problem. He wants the blog post on the ReactComponentRenderer, which includes a setProps and replaceProps helper.

@sebmarkbage Let's figure out the blog post today. I have a couple drafts in the Google doc. Just let me know what you want here and we can put it up.

@spicyj
Member
spicyj commented Jul 14, 2015

@jsfb Not what I read from

as it's not immediately clear how to call React.render on a component that's already been mounted

but let's get the blog post out too. :)

@maoziliang maoziliang referenced this issue in callemall/material-ui Aug 3, 2015
Merged

[RefreshIndicator] Add new component. #1312

@rchanou
rchanou commented Aug 27, 2015

To clarify Functions as Components. Is this just an optimization thing? No JSX style? For example, would I have to use these components like...

<div>
  { Button({ width: 10 }) }
  { Checkbox({ checked: true }) }
</div>
@jimfb
Contributor
jimfb commented Aug 27, 2015

@rchanou It allows for performance optimizations in the core (since we don't need to keep/track instances) and it also means less boilerplate/typing for you (since you just need a function, instead of a fullblown class).

You would still be able to use the component using JSX, just like any other component, it would still be <Checkbox checked={true} />. Of course, you could use the alternate syntax if you liked it better for whatever reason by creating a factory.

@rchanou
rchanou commented Aug 28, 2015

@jimfb Yeah, I want to use JSX :) I just realized I will probably need to use the Babel 5 React options in my webpack babel-loader options, right? Both optimisation.react.constantElements and optimisation.react.inlineElements currently give me Invariant Violation: React.render(): Invalid component element. This may be caused by unintentionally loading two independent copies of React. I'll sort it out...Thanks!

@japgolly

May I suggest that the notes include more detail about lifecycle methods (especially how shouldComponentUpdate will or won't work) with functions-as-components, before the final 0.14 release? For example, if shouldComponentUpdate is provided by React which kind of equality does it use? Cheers.

@spicyj
Member
spicyj commented Sep 10, 2015

Stateless component functions are as if you had no shouldComponentUpdate, or a shouldComponentUpdate that always returns true.

@ericclemmons
Contributor

@spicyj Thanks for the clarification!

So then, if you have a "smart" component that will render depending on nextProps vs. this.props, traditional React.createClass and React.Component are the preferred way to go? Does ES3 modules work as well?

@spicyj
Member
spicyj commented Sep 23, 2015

We checked off this stuff so I'm closing this. @zpao created a separate #4135 and we're also looking at the 0.14 milestone on github.

@spicyj spicyj closed this Sep 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment