search

themeteorchef / base

A starting point for Meteor apps.
http://themeteorchef.com/base
MIT License
689 stars 253 forks source link

Bump version of react-komposer #230

Open Twisterking opened 7 years ago

Twisterking commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

Twisterking commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

themeteorchef commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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.