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

Bump version of react-komposer #230

Open
Twisterking opened this issue Feb 2, 2017 · 13 comments
Open

Bump version of react-komposer #230

Twisterking opened this issue Feb 2, 2017 · 13 comments

Comments

@Twisterking
Copy link

Hi there!

I am currently revamping the UI layer of my production app and switching from Blaze to React.
So I kind of started over with your base.
It occurred to me that you guys are still using the 1.13 version of react-komposer. I am using this is because of the lack of composeWithTracker? Is there any way around this? Maybe providing a helper function which uses something like explained here?
I am really a React noob, so maybe someone experienced can patch this in somehow so we can use the V2.0 of react-komposer?!

On a side note: Why are you not using the vanilla react-meteor-data package?

best, P

@themeteorchef
Copy link
Owner

This is on the list. Just haven't gotten to it yet. Happy to accept a PR if you'd like to take a swing at it!

@tab00
Copy link

tab00 commented Feb 11, 2017

Would it be better to switch to using createContainer() instead, as you've shown in https://themeteorchef.com/tutorials/using-create-container?

@Twisterking
Copy link
Author

I would maybe also suggest to switch to createContainer(), especially as Kadira is closing down and thus also the support for any libraries they created is declining. So maybe it is better to stick to an "MDG in-house solution"?!

@themeteorchef
Copy link
Owner

@Twisterking I've considered this for the exact reasons you've outlined. The gotcha is that createContainer() doesn't allow us to set any options, specifically turning off pure rendering. This is needed because otherwise there's (or at least, was) a bug with React Bootstrap where the navigation active state wouldn't update.

@tab00
Copy link

tab00 commented Feb 13, 2017

specifically turning off pure rendering. This is needed because otherwise there's (or at least, was) a bug with React Bootstrap where the navigation active state wouldn't update.

Was there an issue created for this and if so, has it been solved?

@themeteorchef
Copy link
Owner

I'm not certain. I vaguely recall submitting an issue on this, but it was/has been fairly low priority. It seems like one of the workarounds may have improved (https://github.com/react-bootstrap/react-router-bootstrap) but I haven't tried it.

@stubailo
Copy link

Doesn't createContainer have an option to turn off pure rendering?

@themeteorchef
Copy link
Owner

@stubailo I haven't checked recently but when I first attempted it there wasn't. Checked the source and it was hard-coded. If there's an option that would close this up quick.

@stubailo
Copy link

If you send a PR I'd be happy to merge and release right away! I think the variety of different ways to use React in Meteor is pretty confusing to people and it's a shame to let something that can be changed so easily make the difference.

@themeteorchef
Copy link
Owner

@stubailo playing hot potato, son! This sounds like it's best handled by MDG :) If I can find some time I'll take a peek.

@stubailo
Copy link

Alright, up to you! Personally I think react-komposer is much more confusing for people to use, but of course that's a matter of opinion.

@Twisterking
Copy link
Author

I completely agree with @stubailo! Just today I used both solutions and MDGs createContainer() makes much more sense to me.
btw: Can someone explain to me what exactly "pure rendering" is?!

@stubailo
Copy link

@Twisterking pure rendering is a performance optimization for React - it says that the component should not re-render unless its props or state change.

I think some libraries do sketchy stuff like using setState and then mutating the object and expecting it to render again, but that's not really a recommended way to use React IMO.

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

4 participants