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

docs: Document logic for storing the token in vuex state #1125

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

arlyon
Copy link
Contributor

@arlyon arlyon commented Apr 14, 2021

This is as much an issue as it is a pull request. Currently your token will not appear in vuex with the default prefix (_token.). For me, this is unexpected and at the least should be documented, but may also be a bug.

The "offending" line is:

if (key[0] === '_') {

With the default prefix, the token is not added to the vuex store. At the very least, this should be documented, but I argue that the default prefix should be changed (maybe to just token). What are the consequences of this?

Thanks for the lib! :)

@arlyon arlyon changed the title Document logic for storing the token in vuex state docs: Document logic for storing the token in vuex state Apr 14, 2021
@arlyon
Copy link
Contributor Author

arlyon commented Apr 21, 2021

Not sure how to rerun the semantic commit check but I believe I am compliant. What do we need to merge this?

@codyseibert
Copy link

codyseibert commented May 25, 2021

I am running into this same issue. I don't understand why this if statement is here. I wouldn't think we'd need to change the prefix just to have it show up in the Vuex store. I'm also not seeing it set loggedIn to true either even after changing the prefix to remove the '_'.

@bmulholland
Copy link
Contributor

I recognize I'm getting to this very late, but I appreciate the contribution. I agree that this behaviour isn't obvious, but I don't understand it and we need a better test suite before changing the actual behaviour. For now, having it documented is better than nothing. Thanks again.

@bmulholland
Copy link
Contributor

BTW the semantic PR check failed because the commit itself, not the PR title, didn't match. But all good, I'll squash it.

@bmulholland bmulholland merged commit f4f12dc into nuxt-community:dev Jan 5, 2022
@arlyon
Copy link
Contributor Author

arlyon commented Jan 10, 2022

Ah, of course. That's what I get for coding late at night. Thanks for merging!

@bmulholland
Copy link
Contributor

Thanks for the contribution!

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.

3 participants