Skip to content

Conversation

@petehunt
Copy link
Contributor

This adds some more addons.

If this is accepted we will update the documentation to have instructions for using addons from npm as well as from the premade builds. If using npm you should require('react/lib/ReactTransitionGroup') rather than using react/addons so you don't pull in extra dependencies.

I think we definitely need the event plugins and TestUtils. I wasn't sure about ImmutableObject and StateSetters, but they seem like good ideas.

I considered adding ReactUpdates.injection in there for pluggable batching strategies and ReactPerf, but this already works in npm today (react-raf-batching and react-profiler) and I wanted to keep this as constrained as possible.

If someone signs off on this I'll stack a commit on top with docs (or do it in a new PR, either way).

@petehunt
Copy link
Contributor Author

Also -- I could land react-raf-batching in addons so people will know it's there (@sebmarkbage will probably like this). Is this something people are interested in?

@zpao
Copy link
Member

zpao commented Dec 30, 2013

These are pretty safe but I'm still a little bit concerned about supporting these. I know we've said addons is dangerous and we'll change it at anytime, but I'm afraid we're going to see more copypasta of things that aren't supported.

So I'd like us to make sure we harden our APIs before exposing them as much as possible. Again, these are probably fine (then event hub is the one I'm nervous about) so I think it's fine here

@petehunt
Copy link
Contributor Author

injectEventPluginsByName() should be pretty safe, no?

@vjeux
Copy link
Contributor

vjeux commented Dec 30, 2013

I'm good with it, letting @zpao do the final accept

@zpao
Copy link
Member

zpao commented Dec 30, 2013

Yea, it's more about the case where a component you include injects an event plugin and that's totally outside your control. We're moving quickly into the world where people are using React and we're not having that close contact we had for the first few months. We're not seeing all the ways the uses are unfolding and I'm just getting a bit nervous is all.

Feel free to go for it.

@sebmarkbage
Copy link
Contributor

I like rAF batching and I like add-ons but I wish we didn't have a React with add-ons build. It seems so final and official. It should be easy to expose these. They're really all candidates to be in the core. It's more of a alpha extensions build.

We should take a page out of the old MooTools playbook and allow custom downloads: http://mootools.net/core/
Want even more? Get some more plugins: http://mootools.net/more/

@vjeux
Copy link
Contributor

vjeux commented Dec 31, 2013

I'm regularly sending people towards DOMContainer which has a small yet useful API but we don't provide it. That would be a great addition to addons.

@petehunt
Copy link
Contributor Author

I wonder if we should add PureMixin too? Anything else?

@petehunt
Copy link
Contributor Author

ReactLayeredComponentMixin?

@chenglou
Copy link
Contributor

I was gonna do it for ReactStateSetters, but seeing that the current solution of just creating ordinary methods is enough to get by I think I'll drop this for now. It'd feel like encouraging premature optimization.

@vjeux
Copy link
Contributor

vjeux commented Dec 31, 2013

I don't think it makes sense to expose ReactLayeredComponentMixin without ReactLayer and a couple of behaviors. I think we should punt on this one until we have a good plan for it

@petehunt
Copy link
Contributor Author

I think we could open-source Instagram's fake Layer implementation, since that architecture is not something that's currently in open-source AFAIK. We can let the community expand on it.

Should we add cloneWithProps() too? And mapChildren()? Sheesh :/

@ghost ghost assigned vjeux Dec 31, 2013
@vjeux
Copy link
Contributor

vjeux commented Jan 3, 2014

This pull request is getting out of hand. Let's open a separate pull request per module. It should export the module AND add a documentation for it. We'll be able to discuss on whether we should export modules on a per module basis instead of everyone at once.

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