Skip to content

Commit e23e53a

Browse files
committed
Implements code review suggestions from Victor
1 parent 30b0445 commit e23e53a

File tree

2 files changed

+34
-28
lines changed

2 files changed

+34
-28
lines changed

README.md

+22-17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
![img](http://www.textfiles.com/underconstruction/HeHeartlandBluffs8237Photo_2000construction5_anim.gif)
2-
31
[![Build Status](https://travis-ci.org/GoTeamEpsilon/angular-to-react-redux.svg?branch=master)](https://travis-ci.org/GoTeamEpsilon/angular-to-react-redux)
42

53
![img](http://i.imgur.com/Gq1eJpa.png)
@@ -57,7 +55,7 @@ Scaffolding a project in React/Redux isn't very different from what is typically
5755
└── core.scss
5856
```
5957

60-
Notice how everything is organized in modules rather than a flat approach with directories such as `services/`, `controllers/`, `views/`, etc. This is a best practice that helps one organize a complex user interface while still sharing generic pieces.
58+
Notice how everything is [organized in modules](https://medium.com/@scbarrus/the-ducks-file-structure-for-redux-d63c41b7035c#.ji6r2j61o) as opposed to a flat directory approach. This is a best practice that helps one organize a complex user interface while still sharing generic pieces.
6159

6260
Now that the file structure (hopefully) makes sense, one can go back a directory and run the build tool (you won't find major differences between gulp/grunt and webpack). In our case, it's `grunt serve` and `npm start` for Angular v1 and React/Redux samples, respectively.
6361

@@ -77,7 +75,7 @@ const mapStateToProps = (state) => ({
7775
})
7876
```
7977

80-
One other important thing that is done in this container is the binding of Redux-aware service functions like so:
78+
One other important thing that is done in this container is the binding of Redux-aware service functions (formally known as "action creators") like so:
8179

8280
```jsx
8381
const mapDispatchToProps = {
@@ -89,7 +87,7 @@ const mapDispatchToProps = {
8987
}
9088
```
9189

92-
With these two mappings out of the way, child components can access the state and service functions from above. Here are some highlighted examples of this in `routes/Patient/Demographics/Basic/BasicComponent.js`:
90+
With these two mappings out of the way, child components can access the state and functions from above. Here are some highlighted examples of this in `routes/Patient/Demographics/Basic/BasicComponent.js`:
9391

9492
Displaying data from the store in a table:
9593
```jsx
@@ -129,58 +127,62 @@ handleSubmit(formValues) {
129127
}
130128
```
131129

132-
At this point, you may be thinking _"wait, why are you copying the data from Redux into the local state/form rather than using it directly? Isn't the point of Redux to encapsulate _all_ application state?"_. Good question. As with most things in software engineering, it is always to be able to break the rules when it's justified. Should state such as `{ showForm: true/false }` (determines whether to render the form or not) and `{ cachedForm: this.props.info }` (holds a cache of the form state if the user hits cancel) be put into the Redux store or just be local to the component? It depends. In our case, this state doesn't need to be application wide so Redux is only storing things that are domain-centric rather than domain-centric and UI-centric. These things will often come down to requirements and what the opinions are of your resident seasoned Redux enthusiast(s).
130+
At this point, you may be thinking _"wait, why are you copying the data from Redux into the local state/form rather than using it directly? Isn't the point of Redux to encapsulate _all_ application state?"_. Good question. As with most things in software engineering, it is always best to be able to break the rules when it's justified. Should state such as `{ showForm: true/false }` (determines whether to render the form or not) and `{ cachedForm: this.props.info }` (holds a cache of the form state if the user hits cancel) be put into the Redux store or just be local to the component? It depends. In our case, this state doesn't need to be application wide so Redux is only storing things that are domain-centric rather than domain-centric and UI-centric. These things will often come down to requirements and what the opinions are of your resident seasoned Redux enthusiast(s).
133131

134132
Service Layer
135133
=============
136134

137135
### 🌿 Store
138136

139-
In Angular v1, application-wide state is put into services so that directive controllers can CRUD it. In React/Redux, all application-wide state is put into the store, an object tree. As shown in the above section, components access the store via containers and parent components passing it to them. Components can alter said state by invoking module functions that containers and parent components pass down.
137+
In Angular v1, application-wide state is put into services so that directive controllers can CRUD it. In React/Redux, all application-wide state is put into the store, an object tree. As shown in the above section, components access the store via containers and parent components passing it to them. Components can alter said state by invoking module functions (formally known as "action creators") that containers and parent components pass down.
140138

141139
One key difference between Angular v1 application-wide state in services and Redux store is that state mutation is not allowed. While this sounds weird and scary at first, you _can_ change this state but it must be done in a very specific way. The easiest way to think about this is whenever you update the store, you simply clone the object, mutate the clone to your heart's content, and send that to the store.
142140

143141
Think back to your Angular v1 directives that display information from a service that holds the state. When that service state changes, the directive will change the view to reflect said change. Similarly, Redux-aware React components will change when the store changes.
144142

145143
### ✨ Actions & Pure Reducers
146144

147-
A key difference between the updating of the state in an Angular v1 service and in the Redux store is that you don't "talk" to the store directly. In order to get the store to respond to data changes, you must issue an action. Actions simply send data from your application to your store.
145+
A key difference between the updating of the state in an Angular v1 service and in the Redux store is that you don't "talk" to the store directly. In order to get the store to respond to data changes, you must issue an action. Actions simply send data from your application to your store and then your app "reacts" (pardon the pun).
148146

149147
Recall that the `routes/Patient/Demographics/Basic/BasicComponent.js` calls `this.props.updatePatientData(formValues)` when it wishes to update basic patient information in the store. The `updatePatientData` function is defined in the module `routes/Patient/PatientModule.js` (modules will be covered in the next section) that looks like this:
150148

151149
```jsx
152150
export const updatePatientData = (data) => {
153151
return (dispatch, getState) => {
154152
return new Promise((resolve, reject) => {
153+
console.debug(`updating basic patient data for ${getState().patient.patientInContext}`)
155154
dispatch({
156155
type : 'UPDATE_PATIENT_DATA',
157-
payload : [getState().patient.patientInContext, data]
156+
payload : data
158157
})
159158
resolve()
160159
})
161160
}
162161
}
163162
```
164163

165-
The important piece to focus on for now is the `dispatch` function. This function takes in something called an action. In our case, our action is of type `UPDATE_PATIENT_DATA` and the paloyad is the new basic data with a reference to the patient id that should recieve the update.
164+
The important piece to focus on for now is the `dispatch` function. This function takes in something called an action. In our case, our action is of type `UPDATE_PATIENT_DATA` and the payload is the new basic data.
166165

167166
When the action has been dispatched, something needs to handle it so that the store is updated. This is the job of the reducer. Reducers look at an inbound action and figure out how to update the store with the new information. For example `routes/Patient/PatientModule.js` exposes the following reducer:
168167

169168
```jsx
170169
const initialState = testData
171170
export default function patientReducer (state = initialState, action) {
172171
let result
173-
let copy = clone(state)
172+
let copy
174173
switch (action.type) {
175-
case 'SET_PATIENT_IN_CONTEXT':
174+
case 'UPDATE_PATIENT_IN_CONTEXT':
175+
copy = clone(state)
176176
copy.patientInContext = action.payload
177177
result = copy
178178
break
179179
case 'UPDATE_PATIENT_DATA':
180+
copy = clone(state)
180181
copy[copy.patientInContext].basic = action.payload
181182
result = copy
182183
break
183184
case 'UPDATE_CONTACT_DATA':
185+
copy = clone(state)
184186
const contactIndexForUpdation = _.findIndex(copy[copy.patientInContext].contacts, (c) => {
185187
if (c && c.hasOwnProperty('id')) {
186188
return c.id === action.payload.id
@@ -189,7 +191,8 @@ export default function patientReducer (state = initialState, action) {
189191
copy[copy.patientInContext].contacts[contactIndexForUpdation] = action.payload
190192
result = copy
191193
break
192-
case 'START_ADDING_CONTACT':
194+
case 'INSERT_CONTACT':
195+
copy = clone(state)
193196
const lastContact = _.last(copy[copy.patientInContext].contacts)
194197
let newContactId = 0
195198
if (lastContact != null && lastContact.hasOwnProperty('id')) {
@@ -198,7 +201,8 @@ export default function patientReducer (state = initialState, action) {
198201
copy[copy.patientInContext].contacts.push({ isNewContact: true, id: newContactId })
199202
result = copy
200203
break
201-
case 'DELETING_CONTACT':
204+
case 'DELETE_CONTACT':
205+
copy = clone(state)
202206
const contactIndexForDeletion = _.findIndex(copy[copy.patientInContext].contacts, (c) => {
203207
if (c && c.hasOwnProperty('id')) {
204208
return c.id === action.payload
@@ -232,16 +236,16 @@ export const startAddingNewContact = (data) => {
232236
.then((response) => {
233237
data.id = response.data.id
234238
dispatch({
235-
type : 'START_ADDING_CONTACT',
236-
payload : [getState().patient.patientInContext, data]
239+
type : 'INSERT_CONTACT',
240+
payload : data
237241
})
238242
})
239243
})
240244
}
241245
}
242246
```
243247

244-
You may be thinking _"I see there's a mutation here (function is not pure)... `data` is getting an `id` added on, is that allowable?"_. The answer is "yes". Module functions can be asynchronous and perform have side effects. The important thing is that the reducer that will receive the action will be pure and synchronous.
248+
You may be thinking _"I see there's a mutation here (function is not pure)... `data` is getting an `id` added on, is that allowable?"_. The answer is "yes". Module functions can be asynchronous and have side effects. The important thing is that the reducer that will receive the action will be pure and synchronous.
245249

246250
Unit Testing
247251
============
@@ -262,6 +266,7 @@ Additional Resources
262266
- [1-way vs 2-way Databinding](http://stackoverflow.com/a/37566693/1525534)
263267
- [React Global Error Handling](http://stackoverflow.com/a/31112522/1525534)
264268
- [Redux Logger](https://github.com/evgenyrodionov/redux-logger)
269+
- [Redux Ducks File Structure](https://medium.com/@scbarrus/the-ducks-file-structure-for-redux-d63c41b7035c#.ji6r2j61o)
265270
- [React Logger](https://www.npmjs.com/package/react-logger)
266271
- [ES6 Highlights](https://pure-essence.net/2015/11/29/javascript-es6-highlights/)
267272
- [React/Redux Router Tutorial](https://github.com/reactjs/react-router-redux#tutorial)

samples/react-redux-patient-demographics-example/src/routes/Patient/PatientModule.js

+12-11
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,11 @@ export const setPatientInContext = (patientId) => {
6565
if (!res) {
6666
var message = `Patient ${patientId} doesn't exist`
6767
console.warn(message)
68-
dispatch({
69-
type : 'SET_PATIENT_IN_CONTEXT',
70-
payload : patientId
71-
})
7268
reject(message);
7369
} else {
7470
console.debug(`Setting patient ${patientId} as patient in context`);
7571
dispatch({
76-
type : 'SET_PATIENT_IN_CONTEXT',
72+
type : 'UPDATE_PATIENT_IN_CONTEXT',
7773
payload : patientId
7874
})
7975

@@ -115,7 +111,7 @@ export const deleteContact = (data) => {
115111
return new Promise((resolve, reject) => {
116112
console.debug(`deleting contact data for ${getState().patient.patientInContext}`)
117113
dispatch({
118-
type : 'DELETING_CONTACT',
114+
type : 'DELETE_CONTACT',
119115
payload : data
120116
})
121117
resolve()
@@ -128,7 +124,7 @@ export const startAddingNewContact = (data) => {
128124
return new Promise((resolve, reject) => {
129125
console.debug(`starting to add contact data for ${getState().patient.patientInContext}`)
130126
dispatch({
131-
type : 'START_ADDING_CONTACT',
127+
type : 'INSERT_CONTACT',
132128
payload : data
133129
})
134130
resolve()
@@ -142,17 +138,20 @@ export const startAddingNewContact = (data) => {
142138
const initialState = testData
143139
export default function patientReducer (state = initialState, action) {
144140
let result
145-
let copy = clone(state)
141+
let copy
146142
switch (action.type) {
147-
case 'SET_PATIENT_IN_CONTEXT':
143+
case 'UPDATE_PATIENT_IN_CONTEXT':
144+
copy = clone(state)
148145
copy.patientInContext = action.payload
149146
result = copy
150147
break
151148
case 'UPDATE_PATIENT_DATA':
149+
copy = clone(state)
152150
copy[copy.patientInContext].basic = action.payload
153151
result = copy
154152
break
155153
case 'UPDATE_CONTACT_DATA':
154+
copy = clone(state)
156155
const contactIndexForUpdation = _.findIndex(copy[copy.patientInContext].contacts, (c) => {
157156
if (c && c.hasOwnProperty('id')) {
158157
return c.id === action.payload.id
@@ -161,7 +160,8 @@ export default function patientReducer (state = initialState, action) {
161160
copy[copy.patientInContext].contacts[contactIndexForUpdation] = action.payload
162161
result = copy
163162
break
164-
case 'START_ADDING_CONTACT':
163+
case 'INSERT_CONTACT':
164+
copy = clone(state)
165165
const lastContact = _.last(copy[copy.patientInContext].contacts)
166166
let newContactId = 0
167167
if (lastContact != null && lastContact.hasOwnProperty('id')) {
@@ -170,7 +170,8 @@ export default function patientReducer (state = initialState, action) {
170170
copy[copy.patientInContext].contacts.push({ isNewContact: true, id: newContactId })
171171
result = copy
172172
break
173-
case 'DELETING_CONTACT':
173+
case 'DELETE_CONTACT':
174+
copy = clone(state)
174175
const contactIndexForDeletion = _.findIndex(copy[copy.patientInContext].contacts, (c) => {
175176
if (c && c.hasOwnProperty('id')) {
176177
return c.id === action.payload

0 commit comments

Comments
 (0)