Open johanneslumpe opened 9 years ago
How would one get access to the flux instance in the base component then? Sometimes you want it and you've removed the fluxMixin
that used to put it on this.flux
. FluxComponent
has the render
override that gets flux that you could use to customize how the wrapped component is rendered, but that doesn't make sense for connect
, which is probably often going to be used as a decorator. I mean sure you could manually pull it out of context
but that seems less than optimal.
Also the comment in FluxComponent.js
still says it injects flux into the children's props, so that should be fixed if it's no longer the case...
(Actually reading the code I'm not sure I see where flux is injected into connect
's base component anyway, so this may not be a change in behavior at all...)
@bdowning FluxComponent
does not pass flux
to the render
method anymore. If you check here you will see that the flux
prop is excluded from the props that get passed to render
or to a wrapped child.
You're right about the comment, I'll get that fixed!
As you said, the solution is to pull it from context
if you really require it. By being able to inject actions into your components now, I'm not sure in which case you would really want it.
As for connect
, the flux
prop gets injected here because we are just spreading all props to the wrapped component.
But how does flux
get on the props
of the connect
wrapper component to be passed to the base component on the line you mention? The connect
wrapper calls this.initialize
which sets this.flux
on the wrapper component, but I'm not seeing anywhere it assigns flux
to its props
. It looks to me like flux
will be passed to the base component if and only if it was passed as a prop to the wrapper. Seems like if it's pulled from context instead (as is probably typically the case) it will be absent.
@bdowning I noticed this because I have a connected component as a root component - which receives the flux
prop because I'm passing it in. We should be consistent here. If the FluxComponent
does not pass it to its children, then connect
should not pass it to the base component either.
FluxComponent
is clearly visually a wrapper, so having it throw away props males sense. But when class Foo
uses connect
as a decorator (which sort of implies a fancy class even though it is a HoC under the hood) I think its a bit surprising that some props that you pass to Foo
don't actually make it there.
But then it also doesn't get wrapped statics currently either, so it's hardly transparent.
Personally I'm not sold on the use-HoCs-for-all-the-things mentality and am sort of sad that mixins have fallen out of favor (and fluxMixin in particular has gone away, even though createClass has not) but I acknowledge that I seem to be in the minority here. (Then again I like Common Lisp and CLOS, who's predecessor arguably created the concept in the first place! :). Certainly, though, working with react-router seemed to be a lot easier and more consistent with mixins than with HoCs. But I can always maintain my own fluxMixin if I get desperate.
We removed the flux prop for the
FluxComponent
butconnect
still passes it down. We should remove it.