Skip to content
This repository has been archived by the owner on Aug 5, 2022. It is now read-only.

Week 3: Global state management #3

Merged
merged 4 commits into from
Apr 3, 2019
Merged

Week 3: Global state management #3

merged 4 commits into from
Apr 3, 2019

Conversation

koss-lebedev
Copy link
Contributor

No description provided.

@koss-lebedev koss-lebedev requested review from prichodko and dannytce and removed request for prichodko April 1, 2019 14:21
</Switch>
</React.Fragment>
<Provider store={store}>
<React.Fragment>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need fragment anymore.

Copy link
Contributor Author

@koss-lebedev koss-lebedev Apr 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do, Provider can have only one child element 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @varholak-peter is talking about replacing <React.Fragment> with <>...</> syntax.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe Fragment and <> were released at the same time. Personally, I find Fragment more readable.

Copy link
Contributor

@prichodko prichodko Apr 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think React.Fragment was a workaround until Babel understood <></> syntax.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A always prefer less writing, so <></> syntax for the win! On the other hand, it might be a bit confusing for students 😅


const Cart = connect(mapStateToProps)(CartView)

export { Cart }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not exporting the Cart directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just following the coding style laid out by guys before me, other pages use the same approach.

Copy link
Contributor

@prichodko prichodko Apr 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@varholak-peter, I've been advocating for this approach on my last lecture (it was my personal recommendation and not a requirement, although I would love to keep it consistent in the whole project). Here is a good explanation of that: https://basarat.gitbooks.io/typescript/docs/tips/defaultIsBad.html.


```js
const users.pop() // wrong
const newUsers = users.filter(u => u.id !=== user.id) // right
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!=== 

too many equal signs 😄

@@ -1 +1,69 @@
# Global state management
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add presentation link 👍

@dannytce dannytce merged commit 138e11b into master Apr 3, 2019
@dannytce dannytce deleted the week3 branch April 3, 2019 12:58
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.

5 participants