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

Add Vuex support to vue/no-unused-properties #2276

Merged
merged 8 commits into from
Dec 28, 2023
Merged

Conversation

fuale
Copy link
Contributor

@fuale fuale commented Aug 16, 2023

Changes are aimed at improving Vuex support.

Following Vuex methods will be checked: mapGetters, mapState, mapActions, mapMutations, when no-unused-properties will be enabled

example:

<script>
  export default {
    computed: {
      ...mapGetters(["bar"]),
    },
    methods: {
      baz() {
        return this.bar // no error
      }
    }
  }
</script>
<template>
  {{ baz() }}
</template>

Changes will affect #1675 (Vuex and Pinia support)

@fuale
Copy link
Contributor Author

fuale commented Aug 16, 2023

The issue was created on October 22, 2021, but I am unable to find a way to check Vuex map methods today. To refactor my personal project, I am adding checks now. Maybe it would be useful to you

Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Sorry for the late review. Thanks for your pull request! I have some comments, see below.

lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
tests/lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
@FloEdelmann FloEdelmann changed the title Add Vuex support Add Vuex support to vue/no-unused-properties Sep 15, 2023
@FloEdelmann FloEdelmann linked an issue Nov 14, 2023 that may be closed by this pull request
- reduce duplicated code
- improve error message
- remove duplicated test
@fuale fuale requested a review from FloEdelmann November 22, 2023 17:02
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Some suggestions. Please try them out locally and see if there are any type errors now.

lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
lib/rules/no-unused-properties.js Outdated Show resolved Hide resolved
- add jsdoc
- reuse code from utils
@fuale
Copy link
Contributor Author

fuale commented Dec 4, 2023

Some suggestions. Please try them out locally and see if there are any type errors now.

Done. No type errors.

Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Looks good to me 🙂

@FloEdelmann FloEdelmann requested a review from ota-meshi December 4, 2023 08:40
@CyanSalt
Copy link

It is worth mentioning that Pinia also provides mapWritableState, which may also produce properties.

For reference, eslint-plugin-galaxy created by me once implemented similar functionality (although it may have issues).

Copy link
Member

@ota-meshi ota-meshi 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! Sorry for the late review.

@ota-meshi ota-meshi merged commit a89dd10 into vuejs:master Dec 28, 2023
16 checks passed
@eigan
Copy link

eigan commented Jan 2, 2024

@fuale Thanks for this! Would it be easy to also add support for Pinia mapStores? https://pinia.vuejs.org/api/modules/pinia.html#mapStores 👼🏻

@FloEdelmann
Copy link
Member

I'm afraid that's not possible, since the name of the resulting properties can't be easily deduced.

@eigan
Copy link

eigan commented Jan 2, 2024

@FloEdelmann allright, thanks!

@luizotcarvalho
Copy link
Contributor

luizotcarvalho commented Jan 4, 2024

I really appreciate this feature, thanks @fuale ! While using I found a edge case, Im using Nuxt 2 and mapping a Pinia action to my page but this action is called with a vm inside the beforeRouteEnter like this:

methods: {
  ...mapActions(useStatementStore, {
    resetStatement: '$reset'
  }),
},
...
beforeRouteEnter (to, from, next) {
  next(vm => vm.resetStatement())
}

I keep receiving the error 'resetStatement' of method found, but never used and don't know how to proceed, you think will be needed to ignore cases like this or the support can be broaden a little?

@FloEdelmann
Copy link
Member

@luizotcarvalho please create a new issue following the bug template, so this can be addressed properly.

@Ceall8650
Copy link

Ceall8650 commented Mar 28, 2024

I've upgraded the eslint-plugin-vue plugin to the version: 9.24.0
but the unused mapState still doesn't get the error.
Should I need to upgrade other plugins or some necessary plugins that I need to install?

Here is my package.json

// package.json
...
"vue": "^2.6.11",
"vue-eslint-parser": "^9.4.2",
"eslint": "^7.32.0",
"eslint-plugin-vue": "^9.24.0",
// .eslintrc.js
module.exports = {
  ...
  parser: "vue-eslint-parser",
  parserOptions: {
    parser: '@babel/eslint-parser',
  },
  env: {
    browser: true,
  },
  extends: [
    'plugin:vue/recommended',
    'airbnb-base'
  ],
  plugins: [
    'vue',
    'import',
    'import-helpers'
  ],
  rules: {
    ...,
    "vue/no-unused-properties": ["error", {
      "groups": [
        'props',
        'computed',
        'data',
        'methods',
      ],
      "deepData": true,
    }]
}

@FloEdelmann
Copy link
Member

Please open a new issue with your exact setup and sample code. But before that, please try upgrading your ESLint version, you're a major version behind there.

@Ceall8650
Copy link

OK, Thanks

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.

Vuex and Pinia support
7 participants