search

gertqin / vuex-class-modules

Typescript class decorators for vuex modules
MIT License
194 stars 20 forks source link

Fix inheritance for actions and mutations (#20) #44

Closed seflue closed 3 years ago

seflue commented 4 years ago

Improve tests to expose unexpected behavior in actions and mutations when multiple modules inherit from the same base class.

Implement the minimal fix.

gertqin commented 3 years ago

All the changes look fine!

However I don't like the naming of the mutations/actions in the inheritance tests, as it is impossible to read what the actions/mutations does by its name. This is not your fault, as they were already named this way before your changes (the files somehow got merged without me noticing it), but if you don't mind, feel free to change them to more meaningful names (e.g. action3 -> updateToDoubleFoo or likewise). I can also do it myself after merging the pull request which I'll do after you've decided whether you want to commit your other improvements or not. :)

seflue commented 3 years ago

As I mentioned in the issue, for me it would be the easiest, if you merge the PR first. For additional improvements I would create separate PRs. Because this one was clearly a bug it should get fixed before any refactoring, even if I needed some refactoring before to wrap my head arround the code and identify the problem.