Improve the vue/no-setup-props-destructure rule

[davidmyersdev]

• closes vue/no-setup-props-destructure should also warn when using toRefs #1933
• closes vue/no-setup-props-destructure false negatives in object or array initialization #2007
• closes vue/no-setup-props-destructure should trigger when prop is passed to function #2092

Summary

Previously, the rule did not catch some reactivity-breaking uses of props.

Examples

Accessing a prop in a function call.

<script setup>
  const props = defineProps({ count: Number })
  
  const count = ref(props.count)
</script>

Accessing a prop in an object.

<script setup>
  const props = defineProps({ count: Number })
  
  const counter = { count: props.count }
</script>

Accessing a prop in an array.

<script setup>
  const props = defineProps({ count: Number })
  
  const counts = [props.count]
</script>

Accessing a prop in a nested object structure.

<script setup>
  const props = defineProps({ count: Number })
  
  const counters = [{ counts: [props.count] }]
</script>

[ota-meshi]

Thank you for this PR!

The example reported in #2092 still seems to have false negatives.

<script setup>
const props = defineProps(["foo"]);

function foo(bar) {};

foo(props.foo);
</script>

I think we probably need to add CallExpression listener to check besides VariableDeclarator and AssignmentExpression.

[davidmyersdev]

Thanks, @ota-meshi! That makes sense to me. I should have time to update this within the next few days.

[ota-meshi]

LGTM! Thank you.

[ddenev]

Can someone please explain why is this bad:

<script setup>
  const props = defineProps({ count: Number })
  
  const count = ref(props.count)
</script>

Vue docs use the above as perfectly fine example for initializing a local data property - https://vuejs.org/guide/components/props.html#one-way-data-flow (see 1.):
Quote: 'In this case, it's best to define a local data property that uses the prop as its initial value'.

After the introduction of this change my code stopped working because I'm using this type of local property initialization from props, which was fine for years. And yes, I care only for the initial props value, I do not want to maintain reactivity in the local property.

[davidmyersdev]

I can't speak for the team, but I don't look at it from a perspective of good or bad. I look at this rule as a means to prevent reactivity-breaking code, which your code is technically doing. Just like many other rules, it sometimes makes sense to intentionally break them. If you understand why you want to break reactivity (e.g. only using the value as an initial value), then it's absolutely fine to disable this rule for a given line or file. It's the case where someone does it accidentally that is an issue, and this rule previously did not catch those cases.

[ddenev]

thanks for the reply, @davidmyersdev

I still think that the updated rule is too restrictive. Here are my arguments:

• it is perfectly acceptable (again, Vue docs recommend this) to use props.aProp for initializing a local state
• the rule name contains 'destructure', indicating it was originally taking care of destructuring props, and I am not destructuring props in my code (const count = ref(props.count) is NOT a destructuring)
• since I am aware that I don't want reactivity when initializing local state, I don't want to add loads of 'disable the rule' comments to my code

IMHO, the rule should take care only about destructuring props and not every case where props are used. Currently, it is too intrusive - it forces us to disable it every time we consciously don't want reactivity.

Please reconsider.

Thank you.

[ddenev]

For reference, from Vue documentation:

[richardsimko]

I agree with @ddenev. This rule now does way more than the name and docs implies. I was confused as to why we started getting lint failures all of a sudden and looked up the docs for this rule, it made no sense since no destructuring was made in the code that generated the error. Only after I found this issue did I get an explanation as to why the code failed.

If there is a need for this rule (Reactivity breaking prop access) please make it a different rule or rename the existing rule.

And in the future when making breaking changes to rules, please consider respecting semver and releasing it as a major version.

[l3d00m]

I've opened #2259 since I strongly agree with @ddenev and @richardsimko

[volarname]

i strongly agree with a completely new opt-in rule instead of this update because vue/no-setup-props-destructure its not correct for this, and i will absolutely opt-out this rule, because in some cases you want the prop as initial non-reactive value for the component

const props = defineProps(['initialCounter'])

const counter = ref(props.initialCounter)

this is a correct code, also in vue docs

[FloEdelmann]

Let's please discuss this in #2259, I'll lock the discussion here now.