Skip to content
This repository has been archived by the owner on Oct 24, 2021. It is now read-only.

Some additions to the Meteor/React guide #263

Closed
wants to merge 1 commit into from
Closed

Some additions to the Meteor/React guide #263

wants to merge 1 commit into from

Conversation

ffxsam
Copy link
Contributor

@ffxsam ffxsam commented Mar 8, 2016

Open to discussion of course! People might want to learn more about Redux, what it is, if they need it, what advantages it brings to the table. Performance testing is a biggie, because components can often go on a re-render binge and cause problems. These tools can help measure and pinpoint trouble spots.

Open to discussion of course! People might want to learn more
about Redux, what it is, if they need it, what advantages it
brings to the table. Performance testing is a biggie, because
components can often go on a re-render binge and cause problems.
These tools can help measure and pinpoint trouble spots.
@tmeasday
Copy link
Contributor

tmeasday commented Mar 9, 2016

Hey @ffxsam I think for now we'll leave off covering non-Meteor-specific React content (unless you are feeling excited to write it!).

I think covering how to use Redux with Meteor would be quite interesting; I'd be excited to see a bit more of a summary of what you might cover?

@rhyslbw
Copy link
Contributor

rhyslbw commented Mar 9, 2016

Also interested in this

@ffxsam
Copy link
Contributor Author

ffxsam commented Mar 9, 2016

Totally crunched for time at the moment, but let's leave this open for now. I'll come back when I have more time and we can talk about it

@tomRedox
Copy link
Contributor

@tmeasday I've just been through the loop of learning Redux and how to integrate it with Meteor. The bit I found conceptually difficult was deciding how much, if anything, from the local minimongo store to bring into state.

I wanted to be able to see how changes in the UI effected the data and to time travel on that, so I ended up copying the record being edited into state, but leaving the rest of the minimongo data (the data in lists that wasn't being edited for example) out of the state.

This is the code I came up with for that: https://github.com/tomRedox/simpleCRM/tree/master/imports/ui/redux. If you think that's a good approach I'd be happy to write it up. If you don't, I'd be interested to hear your thoughts on a better one and I can incorporate it into that example app.

My code was based on Abhi Aiyer's How we Redux series of articles, so he might be another good person to talk to about writing some words on Redux integration

@tmeasday
Copy link
Contributor

cc @abhiaiyer91

@tmeasday
Copy link
Contributor

Thanks @tomRedox, I'll take a look at that soon

@ffxsam
Copy link
Contributor Author

ffxsam commented Mar 10, 2016

I only took a quick glance, but I would make a few suggestions with regards to conventions/syntax.

For thunks, IMO it's best to have a pattern like this:

  • Thunk: USER_REQUEST_[action name]
    • Succeed: USER_[action name]_SUCCEEDED
    • Failed: USER_[action name]_FAILED

Also, I might have missed something (likely, since I didn't look in detail), but I'm not sure why you're using thunks this way:

https://github.com/tomRedox/simpleCRM/blob/master/imports/ui/redux/customer-list-actions.jsx

.. when there's no async actions being performed?

@tomRedox
Copy link
Contributor

@ffxsam that's great feedback, thank you. I was a bit lost in the thunks to be truthful, I need to revisit it. I was trying to move the dispatching out of the container, but the code felt like overkill as I was typing it, I'd very glad to find a way to slim it down.

@ffxsam
Copy link
Contributor Author

ffxsam commented Mar 10, 2016

You're welcome! Redux (IMO) although simple, takes a while to fully grasp. I would keep all Redux dispatches in your container components, and pass stuff down to children via props. It keeps your code more maintainable.

Here's an example of regular action creators and thunks.

Though I should follow my own advice and rename those actions to USER_REQUESTED_SAVE_COLOR, USER_SAVE_COLOR_SUCCEEDED, USER_SAVE_COLOR_FAILED.

The USER_ prefix also helps you know right away that this action is something invoked by the user, not some other action that the user did not initiate (such as a timer expiring).

@tomRedox
Copy link
Contributor

Thanks @ffxsam that's great, I'll have a look at changing the code tomorrow or over the weekend

@abhiaiyer91
Copy link

I was plannign on writing an article for Client Side Data Management. But havent found the time

@tmeasday
Copy link
Contributor

Yeah! I mean Redux isn't necessarily React specific, I think the ideal situation would be a Client Side Data article (thanks for reminding me Abhi!).

I'm not sure how quickly we'll get to it so I think this'd be a great one for external folks to work together on.

@tmeasday
Copy link
Contributor

Perhaps we can take this discussion here: #234

@tomRedox
Copy link
Contributor

@ffxsam I've remembered why I was doing the thunks the way I was, I've added some notes to this issue that explain my reasoning: tomRedox/simpleCRM#19. I'd really appreciate your thoughts if you had a moment at some point.

@Joe312341
Copy link

Hello,

As per the discussion here adding an example for redux would be very helpful.

It would show a concrete example of how to implement React using Redux in Meteor. Could be treated as an appendix to the guide.

In addition sample apps implementing this architecture would be useful. They would have to definitely include testing though. I think creating a collections of apps like the one from @tomRedox could serve as good foundation if enhanced a bit more. But the most important part here is that they are sanctioned by MDG itself.

Currently we just have a few apps that aren't fully built out (for example no testing) and are also not sanctioned by meteor and I think this leaves people with a lot of questions

@tmeasday
Copy link
Contributor

@Joe312341 - have you checked out https://github.com/meteor/todos/tree/react ?

@tmeasday
Copy link
Contributor

@ffxsam what is your latest thinking here?

@ffxsam
Copy link
Contributor Author

ffxsam commented Jun 28, 2016

Funny how things change! :) I actually vote no on Redux. It's definitely useful for certain things like maintaining global UI state (e.g. "is the notification pane open or closed"). But in most cases, one can get away with just relying on React component state for things.

On the other topics:

  • Pre-binding: yes, absolutely. In fact, I've created a package to address this.
  • React Perf Tools: nice to have. I don't know if I've ever encountered any issues that it pointed out.. maybe once or twice. I don't know if it merits inclusion in the Meteor Guide though, honestly.

@lorensr
Copy link
Contributor

lorensr commented Jun 28, 2016

Thanks @ffxsam – moved those two items to #492 and #493. Would be happy to look at a PR. And there are some more React article improvement ideas in #397

@lorensr lorensr closed this Jun 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants