'Strict' option for vue/no-unused-properties

[dsl101]

What rule do you want to change? vue/no-unused-properties

Does this change cause the rule to produce more or fewer warnings? More when strict: true

How will the change be implemented? (New option, new default behavior, etc.)? I have argued that this should be the default behaviour, in line with externally-called methods being considered 'unused' unless explicitly labelled in JSDoc style. But, if this change is undesirable, a strict: true option to vue/no-unused-properties might be a compromise, with the possibility of making strict: true the default in future.

Please provide some example code that this change will affect:

<script>
  export default {
    data () {
      return {
        unusedData: 42
      }
    },
    computed: {
      notReferenced () {
        return 'never seen'
      },
    },
    mounted () {
      const handlerName = 'eventHandler'
      // Event registration code ommitted, but would end up calling via:
      this[handlerName]
    },
    methods: {
      eventHandler () {
        // Used
      },
      notused () {
        // should show as warning
      }
    }
  }
</script>

What does the rule currently do for this code? The presence of this[handlerName] (as noted in the original ticket) makes it almost impossible for static analysis to determine whether any particular method or property is actually used. The current behaviour appears to assume that all properties, methods, data, and computeds are therefore used, and the above code produces no warnings at all. Commenting out this[handlerName] restores the (in my opinion, correct) behaviour that only explicitly referenced properties are 'used'.

What will the rule do after it's changed? Warn about any properties not explicitly referenced, or which are labelled as referenced by the developer using either // eslint-disable-next-line vue/no-unused-properties or /** @public */ annotations.

[ota-meshi]

I don't like the option to change the way it works, but I don't reject to add the option to change the way it works.

However, the name of the option can be confusing. I'd like to have an option name that makes it easy to understand how to handle properties that can't be tracked.

[dsl101]

I'm not precious about the name of the option—strict came to mind simply because of the analogy with JS use strict and the fact it turns on a more strict definition of 'unused'.

explicit is an alternative? When true, only explicitly referenced properties are considered used? Did you have something in mind that's less confusing?

[ota-meshi]

There are some patterns that this rule considers to have used all properties.

e.g.

<script>
  /* eslint vue/no-unused-properties: [2, {groups:['data']}] */
  export default {
    data () {
      return {
        unusedData: 42
      }
    },
    methods: {
      fn () {
        return this
      }
    }
  }
</script>

I think we need to allow the user to set whether to consider each as using all properties or not.

The option we add is an option that controls the decision to access an unknown property, so I want the option name to be descriptive. But I can't think of a good name 😓.

[dsl101]

I'm not sure I completely follow what you were trying to illustrate there—did you mean this new setting should be per group? If so, how about assume-unused: ['data'] to indicate that any data element not explicitly referenced should be assumed to be unused.

[dsl101]

Although, I'm now not seeing the point of including 'data' in the groups: setting—and I'm back to why this isn't really just a bug. If I don't want to be warned about unused props, data, etc., I would just leave them out of the groups: set. Maybe I misunderstood what your last post was meaning...

[ota-meshi]

I wanted to say that the last example I wrote would not be warned by return this. If you comment out return this, unusedData will be warned.

You can try it DEMO.

[ota-meshi]

For example, if the option name was strict, it would be confusing to the user how to handle return this.

[dsl101]

When you say ”how to handle return this“, what are you expecting them to do?

I still argue that warning about unused properties according to the existing settings should not be affected by the code containing a line like return this, and thus this really is a bug. But if you’re still keen that this current behaviour is correct, then what I’m after is a stricter interpretation of ‘unused’. I’m not sure what’s confusing about that as an option name.

[ota-meshi]

I was misunderstanding your option suggestions, but I probably understood what you were saying. However, I don't agree with naming the option strict. Strictly speaking, all reports for this rule are false positives as all properties are accessible via $refs etc. (for Vue<3.2 or without expose). So I think it hard to understand what strict means.

The current behavior is implemented to be false negatives to avoid false positives as much as possible when the property usage becomes untraceable. I think it's a user's preference how to handle some of these processes, so I think it should be possible to set each one.

For example, how about adding the following option to control known processes?

{
  "vue/no-unused-properties": ["error", {
    "stopReporting": [ // To avoid false positives, stop reporting if the specified process is found.
      "unknownPropertyAccess", // e.g. this[unknown]
      "returnInstance", // e.g. return this
      // There may be other than these.
    ]
  }]
}

I think the settings you want are:

{
  "vue/no-unused-properties": ["error", {
    "stopReporting": [] // Probably "strict" for you as it doesn't stop the reporting.
  }]
}

What do you think? (I'm not good at English, so option names can still be confusing to people 😓)

[ota-meshi]

If anyone wants to work on this change, open a PR. If no one works for a long time, I will close this issue.

[dsl101]

Right, so there would be a default value of stopReporting which would match the current behaviour, and by setting that to an empty array, it would not stop reporting for any ambiguous (e.g. this[handlerName]) reference? I think I follow, and that sounds like it would do what I'm after.

I don't think I know the internals well enough to start on this, so hopefully some kind soul will jump in :).