arunoda / react-komposer

Feed data into React components by composing containers.
MIT License
732 stars 70 forks source link

meteor example needs improvement #132

Open TobyEalden opened 7 years ago

TobyEalden commented 7 years ago

I'm guessing lots of meteor users will be coming here hoping to quickly understand how to integrate/port existing code to v2. The example in the readme isn't much help IMO (and it's buggy - trackerCleanup is undefined and will throw in the example given).

I have improved the example to be more in keeping with the v1 syntax - see https://github.com/arunoda/react-komposer/pull/131

arunoda commented 7 years ago

I've update the README, try using that. Thanks for the heads up.

stewartcelani commented 7 years ago

Also coming from the Meteor + React + React-Komposer world.

Keen to upgrade to v2. Have a product in beta at the moment using v1. Failing to make sense of your examples. Has Meteor fallen down in the list of your priorities or... ?

Unsure how to convert below code into v2. My whole app depends on react-komposer but the bugs encountered in v1 do lead to less performance than I'd like (a teacher viewing 30 student screens at the same time -> 1 update on a single student somehow triggers all composer functions).

const mapStateToProps = (props, onData) => {
    console.log("App.jsx -> mapStateToProps")
    let state = store.getState()
    let theme = state.theme
    console.log(state)
    onData(null, {...state, theme})
    return store.subscribe(() =>{
        let state = store.getState()
        let theme = state.theme
        onData(null, {...state, theme})
    })
}

function mapMeteorToProps(props, onData) {
    console.log("App.jsx -> mapMeteorToProps")
    if (!Meteor.user()) { return }
    if (Meteor.subscribe("ViewerPreferences").ready() && Meteor.subscribe("Categories").ready() && Meteor.subscribe('Viewers').ready() && Meteor.subscribe("Groupmemberships").ready() && Meteor.subscribe("User").ready() && Meteor.subscribe("SettingsPublic").ready()) {
        let viewerId = ''
        try {
            viewerId = Meteor.user().mail.toLowerCase()
        } catch (e) {} // throws error on logout
        const viewer = Viewers.findOne({_id: viewerId})
        const groups = Groupsmembers.find({email: viewerId}, {sort: { groupId: 1}}).fetch()
        const user = User.find({}).fetch()
        const savedPreferences = ViewerPreferences.findOne({_id: viewerId}) || false
        const appSettings = Settings.findOne({_id: 'ldap'})
        onData(null, {viewer, groups, user, viewerId, savedPreferences, appSettings})
    }
}

const noLoading = () => (<div></div>)

export default composeAll(
    compose(mapStateToProps, noLoading),
    composeWithTracker(mapMeteorToProps, noLoading),
)(App)`
stewartcelani commented 7 years ago
function getReduxLoader(mapper) {
    return (props, onData, env) {

The returns in the examples (e.g. For Promises & For Redux) are also missing the => and don't execute:

Fix:

function getReduxLoader(mapper) {
    return (props, onData, env) =>  {
TobyEalden commented 7 years ago

I gave an example of a drop-in replacement of composeWithTracker that works for v2

@arunoda - I'd be interesting to know why you don't recommend this approach, as it is a simple shim around compose and doesn't break the pattern IMO.

stewartcelani commented 7 years ago

Nice work Toby. I've already ported every single one of my components (fun times) but this should probably be used by default to make the transition easier for everyone.

Have you managed to get redux working as I reference in #134?

TobyEalden commented 7 years ago

@stewartcelani - I'm afraid not, haven't looked at that yet. Good luck!

arunoda commented 7 years ago

@TobyEalden about the composeWithXXX pattern. I'd like to go with dataLoader generators instead of composeWithXXX. Here's the problem why I don't recommend.

With v2, we highly suggest you to create your own compose function. So, inside the composeWithXXX function it don't know what compose to use unless you pass it explicitly. Specially, if that composeWithXXX distributed via NPM.

So, dataLoader generator makes a lot of sense.

arunoda commented 7 years ago

@stewartcelani

For you example, you could do this like this without much effort. (You need to import getTrackerLoader as I present here.)

// all the existing code you've

const loadingHandler = () => (
   <div>Loading...</div>
);

export default composeAll(
    compose(mapStateToProps, { loadingHandler }),
    compose(getTrackerLoader(mapMeteorToProps), { loadingHandler })
)(App)`
akashqwerty commented 7 years ago

Hi @arunoda Sir, I am having trouble using react-Komposer with meteor. I am unable to subscribe data . I have tried MDG's createContainer method and It worked. I have developed for one year using Blaze and now I am migrating to react but facing a lot of problems. Do I need redux too when using react-Komposer? If any of you may please provide a simple working sample of how to retrieve data from collection and display it using react Komposer that would be highly appreciated. My working react version is 16. I have already tried method provided by https://themeteorchef.com/tutorials/using-react-komposer , but having hard luck. Thank you.