Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vuex 101 Nano #51

Merged
merged 3 commits into from Jan 6, 2019
Merged

Vuex 101 Nano #51

merged 3 commits into from Jan 6, 2019

Conversation

marina-mosti
Copy link
Contributor

MD file for nano1-vuex

Copy link
Contributor

@NataliaTepluhina NataliaTepluhina left a comment

Choose a reason for hiding this comment

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

Thank you so much for working on this, @imMarina! This is a great nano
I've added some comments/suggestions (debatable ofc)
Keep up the good job! 👍


## 2. Step 1 - Installing Vuex

Since we are using codepen, the only thing we need to install Vuex is to add it to our list of
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think on renaming codepen to sandbox here? It might be confusing to people who use CodePen vs CodeSandbox

## 2. Step 1 - Installing Vuex

Since we are using codepen, the only thing we need to install Vuex is to add it to our list of
dependencies, so go to the `Dependencies` button on the bottom left of your screen and add it to
Copy link
Contributor

Choose a reason for hiding this comment

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

In fact, it's Dependencies tab with Add Dependency button :)

el: "#app",
components: { App },
template: "<App/>",
store
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add a note that this is ES6 shortcut for store: store?

In this case, we are using an **action** via *dispatch* to add a game to our store.

::: tip 💡
*Why am I not mutating the data with a commit?* It is a best practice to only commit from within actions.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would disagree with this statement. Vuex docs doesn't have a strict recommendation for this as a best practice and Vue core team has different positions on this. While I personally support using only actions on component level, let's make the statement on best practices a bit softer: it's considered as a one of good practices for example. We can mention that it fact there is nothing bad in committing mutations directly when we have sync stuff but actions approach is more maintainable.

workshop/nano1-vuex.md Show resolved Hide resolved
which will allow us to modify the whole array on one go.

::: tip 💡
Notice that we are using ES6 object destructuring in the first param. This way instead of having to pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a link to destructuring

Suggested change
Notice that we are using ES6 object destructuring in the first param. This way instead of having to pass
Notice that we are using [ES6 object destructuring](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Object_destructuring) in the first param. This way instead of having to pass

This only scratches the surface of Vuex, and an ideal scenario would be to use it on a multi-component
app where the state needs to be shared.

Try adding a new component Form.vue in which you add new games to your library using what you have learned!
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Try adding a new component Form.vue in which you add new games to your library using what you have learned!
Try adding a new component `Form.vue` in which you add new games to your library using what you have learned!

@NataliaTepluhina
Copy link
Contributor

@jlooper maybe you can review the workshop as well?

```

Here we grab a copy of our `state.games` array into a `games` constant.
(It's important that we make a copy, since we should **NEVER** modify our state directly!).
Copy link
Contributor

Choose a reason for hiding this comment

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

I love how you explained the consept of not changing state outside of mutation! Good job 👍

our project.

::: tip 💡
When working on a local project, you should first add the dependency with `npm install vuex` or
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome point for people who want to work not only in sandboxes! Must-have for all advanced nanos 👍

@jlooper
Copy link
Contributor

jlooper commented Jan 6, 2019

This is very cool! Thanks for this great contribution. Vuex is one of those hard-to-understand concepts that you've explained very clearly!

I'd like to review/copyedit the next iteration. I recommend making Natalia's suggested changes, then I will go through and edit for style/clarity, sound good?

@marina-mosti
Copy link
Contributor Author

@jlooper @NataliaTepluhina I've adjusted based on Natalia's comments. We can go with style/clarity now if you like as you suggested

Copy link
Contributor

@jlooper jlooper left a comment

Choose a reason for hiding this comment

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

These are looking good to me. I would like to merge this nano in and then copyedit

@jlooper jlooper merged commit cce7c4b into master Jan 6, 2019
@jlooper jlooper deleted the feature/mm/nano-vuex branch November 8, 2019 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants