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

Add docs for context #580

Closed
sophiebits opened this issue Nov 21, 2013 · 18 comments
Closed

Add docs for context #580

sophiebits opened this issue Nov 21, 2013 · 18 comments

Comments

@sophiebits
Copy link
Collaborator

No description provided.

@jaredly
Copy link
Contributor

jaredly commented Mar 19, 2014

Is this still "undocumented on purpose"?

@zpao
Copy link
Member

zpao commented Mar 20, 2014

Yes. I need to add a project about cleaning up context so we can make sure this is usable.

@ericwooley
Copy link

I would like to bump this, maybe it's undocumented on purpose, but people are using still using it, and it can be quite painful. Especially when you are trying to upgrade react and aren't sure if contexts as a feature changed or if your libraries are breaking for other reasons.

If it's not documented on purpose, can we at least put warnings in when people are using them, saying that either they shouldn't be using them until they are officially supported, or to be careful using them as they are likely to change.

Also, if they do change, it would be nice to be able too see how they worked at older versions, in case you need to work on an old project.

@gaearon
Copy link
Collaborator

gaearon commented Sep 24, 2015

If React warned on context usage, it would essentially be impossible to use because nobody wants to use a library causing React to output junk warnings.

@jimfb
Copy link
Contributor

jimfb commented Sep 24, 2015

@ericwooley They aren't documented, and I think we've been pretty consistent about telling people that it's not a supported feature and should not be used yet. We try to add warnings when we change context in a breaking way. But they are undocumented intentionally. We can't add a global warning because as @gaearon said, it would cause libraries like react-router to start emitting warnings in everyone's projects.

@sophiebits
Copy link
Collaborator Author

it would essentially be impossible

Is that a bad thing? ;)

@ericwooley
Copy link

If React warned on context usage, it would essentially be impossible to use because nobody wants to use a library causing React to output junk warnings.

IMO that would be perfect, hopefully the package maintainers will figure out a way to accomplish things without depending on unsupported and undocumented features. Weather its officially supported or not, people are using it in major packages and the lack of documentation is a hindrance. Warnings would discourage its usage in packages, but I can see why that wouldn't be desirable, as several large packages use it, which brings me back full circle to contexts needing documentation.

As for not documenting it because it's not supported, that doesn't make sense to me. Why not just put a big warning in the documentation that it's not supported and then document how it works at this point in time.

@ffxsam
Copy link

ffxsam commented Sep 27, 2015

Oh god.. please oh please oh please, keep this feature in React! It's saving my hide big-time on a project, and allowing me to easily let my components communicate without passing props all over the place, and without having to add complexity to my app by adding something like Flux or Redux.

The only issue I've noticed (using 0.13), is that if you use React.cloneElement, context is not preserved. So I'm using React.addons.cloneWithProps which works. I read somewhere that in 0.14, React.cloneElement will properly handle passing context downward.

@gaearon
Copy link
Collaborator

gaearon commented Sep 27, 2015

Is that a bad thing? ;)

If there's a better way to implement something like <Router> <-> <Link> in React Router or <Provider> <-> connect() in React Redux without resorting to singletons, please tell me. :-) I'd very much like to drop context usage but I can't find a way to implement server rendering with independent requests while keeping a sane component API.

@ffxsam
Copy link

ffxsam commented Sep 27, 2015

I'd very much like to drop context usage

PLEASE DON'T. It will ruin lives. I'm building my SaaS in Meteor, and there's no clean/simple way to include a Flux-like architecture in Meteor at the moment.

@gaearon
Copy link
Collaborator

gaearon commented Sep 27, 2015

PLEASE DON'T.

You're free to use it in your code :-). I'm just saying I've been having troubles with it being half-baked, and I would gladly avoid it if I knew how.

IMO that would be perfect, hopefully the package maintainers will figure out a way to accomplish things without depending on unsupported and undocumented features.

As a package maintainer, I can assure you that both me and many people I talked to (e.g. React Router folks) wouldn't touch context with a ten foot pole if it was possible to figure out a way to accomplish some important things (mostly related to server rendering and sideway data) without it. Unfortunately, to the best of my knowledge, context is the only solution to some important use cases right now.

@ffxsam
Copy link

ffxsam commented Sep 27, 2015

I'd love to jump into Redux, but the problem right now is that with Meteor, you have to use something like browserify or webpack to include it (as there's no nicely packaged Meteor package for Redux), and as of Meteor 1.2, I've run into some issues getting browserify and the npm container working.

Just curious, what do you find half-baked about context? So far it's smooth sailing for me. The only annoying thing I've run into is having to maintain duplicates of all the context items at the top level, one in childContextTypes to define the type, and one in getChildContext() to link it up.

@gaearon
Copy link
Collaborator

gaearon commented Sep 27, 2015

as there's no nicely packaged Meteor package for Redux

If it helps, there's a UMD build at cdnjs: https://cdnjs.com/libraries/redux
(We're off topic. If you'd like to discuss Redux, it's better to do in its issues instead.)

Just curious, what do you find half-baked about context?

For example, this: #2517

@ffxsam
Copy link

ffxsam commented Sep 27, 2015

For example, this: #2517

A bit beyond my comprehension, I'm afraid, as I'm somewhat new to React.

Going forward with 0.14 and beyond, I think it would be cool for React to have some sort of built-in easy way for components that are quite far apart (child and great-great-great-grandparent) to communicate—whether context is polished up, or something else replaces it.

@mj12albert
Copy link

@ffxsam

I think it would be cool for React to have some sort of built-in easy way for components that are quite far apart (child and great-great-great-grandparent) to communicate

I’m also trying to do this with Meteor – was considering using context for storing window width/media queries but this thread is making me uncertain whether I should, or just go with something like this: https://facebook.github.io/react/tips/dom-event-listeners.html (which might be safer? but very un-DRY)

@samuelsimoes
Copy link

At first glance I was really convinced to use this context to create some single entry point to action creators setup on my components, something like was suggested on this post http://jaysoo.ca/2015/06/09/react-contexts-and-dependency-injection/ too. With this, on a "setup component", I can see all actions creators used on my app (I have one react app per page here). Ok, I could pass the actions downward on the props, but with many nested components it'll be very painful. It will be sad if this feature is removed.

@ffxsam
Copy link

ffxsam commented Oct 2, 2015

FYI, I totally changed direction on this and stopped using context as it's likely to be dropped. Not to mention I hit an infinite loop issue with it - so I feel like maybe it's a half-baked solution.

I'm building in Meteor, so I'm using the meteorflux:dispatcher package and it's working great for me.

@zpao
Copy link
Member

zpao commented Oct 2, 2015

We should document this and control the messaging. It's too late to pretend
that React doesn't currently support context. At least this way we can
document the caveats and talk about whatever future plans might be. I'm
tired of linking to a couple issues and fielding questions based on what
people are reading around the Internet.

On Friday, October 2, 2015, Sam notifications@github.com wrote:

FYI, I totally changed direction on this and stopped using context as it's
likely to be dropped. Not to mention I hit an infinite loop issue with it -
so I feel like maybe it's a half-baked solution.

I'm building in Meteor, so I'm using the meteorflux:dispatcher package
and it's working great for me.


Reply to this email directly or view it on GitHub
#580 (comment).

sophiebits added a commit to sophiebits/react that referenced this issue Oct 9, 2015
sophiebits added a commit to sophiebits/react that referenced this issue Oct 9, 2015
sophiebits added a commit that referenced this issue Oct 9, 2015
Fixes #580.

(cherry picked from commit 28b10a9)
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

9 participants