graphistry / falcor

Graphistry forks of the FalcorJS family of projects in a lerna-powered mono-repo.
23 stars 3 forks source link

falcor-react-redux: connected HOC does not receive parent props #9

Closed jameslaneconkling closed 7 years ago

jameslaneconkling commented 7 years ago

In cases like this:


const ComponentWithFalcor = connect(SomeComponent);

render((
  <ComponentWithFalcor someProp="value" />
), document.getElementById('app'));

the inner SomeComponent won't receive the prop someProp. It looks like this is b/c mergeReduxProps (here) only passes falcor from the third ownProps argument.

Unless I'm missing something, mergeProps isn't necessary here. I tried changing to

connectRedux(mapReduxStoreToProps, null, null, reduxOptions),

and it seemed to work. Haven't tested properly, though, b/c I haven't gotten the lerna build working...

trxcllnt commented 7 years ago

@jameslaneconkling this is legacy from when we were doing redux integration differently. I don't have time now to go into details, but will follow-up later with an explanation. In the meantime, feel free to submit a PR that merges the parent props in too. BTW, what issues are you having with the lerna build?

jameslaneconkling commented 7 years ago

Submitted PR #11

Although after checking out falcor-react-schema, I see that that implementation doesn't even use the redux connector, so this bug wouldn't be an issue.

I'll take a longer look at falcor-react-schema. B/c it isn't connected directly to the redux store, it doesn't presume a specific shape for the redux store, which seems like a good thing. If it gets to a point where it's shareable and you have time to whip up an example, please do.

As for lerna, I couldn't figure out the lerna equivalent of npm link for local development, though admittedly I didn't look hard.

trxcllnt commented 7 years ago

@jameslaneconkling re: lerna, from the root of the git project (after doing npm install) you can do npm run bootstrap to link up all the projects. You can also run other commands, like npm run test-coverage, which will execute the test-coverage script in each of the packages that have them.