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

Should vuex mapState print error message #1093 #1162 #1297

Merged
merged 1 commit into from
Nov 10, 2019
Merged

Conversation

EdyHartono
Copy link
Contributor

  • return an empty array when map is not valid
  • log an error message when map is not valid

@EdyHartono EdyHartono changed the title Should vuex mapState print error message #1093 Should vuex mapState print error message #1093 #1162 Jun 3, 2018
@EdyHartono
Copy link
Contributor Author

EdyHartono commented Jun 3, 2018

@ktsn i don't know if i should directly throw an error in non production mode or just log the error, any suggestion ?

@joachimboggild
Copy link

I think it would be nice if it threw an error. As far as I can see, if a developer is not providing the proper values, he is making a mistake, and it would be a bigger help to actually have this exception being thrown, because it would be more noticeable.

Another small suggestion for this phrasing:
"[vuex] states parameter must be either an Array/Object"

It might sound better with
"[vuex] states parameter must be either an Array or an Object"
or
"[vuex] states parameter must be Array/Object"

"either" sounds a bit strange together with the "slash" phrasing.

... just my two cents :-)

Thanks for doing awesome work.

@jamescostian
Copy link

This PR would have saved me an hour today if it were merged. Is there any reason why it hasn't been merged? It's a fairly small and simple PR and it provides Vuex users with much better feedback than is currently provided when you make a mistake with mapState

@sustained
Copy link

Also wondering why this wasn't merged.

Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@ktsn ktsn merged commit e5ca2d5 into vuejs:dev Nov 10, 2019
@vue-bot
Copy link

vue-bot commented Nov 10, 2019

Hey @EdyHartono, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚

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.

7 participants