`vue/no-setup-props-destructure` is too restrictive after latest update

[l3d00m]

What rule do you want to change?

vue/no-setup-props-destructure

Does this change cause the rule to produce more or fewer warnings?

fewer

How will the change be implemented? (New option, new default behavior, etc.)?

Revert #2244 and extract those rule changes into a new rule.

The changes now cause rule violations for example code directly from the vue documentation (see #2244 (comment)).

It now enforces more or less subjective defaults. I mostly agree with the defaults, but the rule is active in vue3-essential, which shouldn't happen for subjective defaults.

In addition to that, the name and documentation no longer matches with the behaviour.

Please provide some example code that this change will affect:

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

What does the rule currently do for this code?

The code is violating the rule, even though this is not a destructure, but a non reactive access.

What will the rule do after it's changed?

It shouldn't violate. This is a valid use case for setting an initial value, even though it might not be required to do so often. Still, it shouldn't be necessary to disable the rule for every line where this occurs.

Please consider making this a new rule, which could be placed in recommended instead. This has nothing to do with destructuring anymore, which is indeed critical and should fail.

[websmurf]

We are also running into issues with the last updated version, for example:

<script lang="ts">
export default defineComponent({
  props: {
    canSave: {
      type: Boolean,
      default: true,
    },
    showSave: {
      type: Boolean,
      default: true,
    },
  },
  setup (props) {
    const disabled = ref<boolean>(props.canSave === false)
                                  ^^ Failing

or

<script lang="ts">
export default defineComponent({
  props: {
    id: {
      type: String,
      required: true,
    },
    modelValue: {
      type: Boolean,
      required: true,
    },
    noLabel: {
      type: String,
      required: true,
    },
    yesLabel: {
      type: String,
      required: true,
    },
  },
  emits: ['update:modelValue'],
  setup (props, { emit }) {
    ...
    const options: RadioInputItem[] = ref([
                                      ^^ Failing
      { value: false, label: props.noLabel },
      { value: true, label: props.yesLabel },
    ])

[daniluk4000]

Same. We use prop value as default value for some refs. It would be nice if we could exclude this case via additional option.

[l3d00m]

It would be nice if we could exclude this case via additional option.

Please, do not make this an additional option. As stated in the issue, the linting is completely unrelated to what vue/no-setup-props-destructure implies so it should be a new rule, rather than an option.

It might be a good idea to revert the #2244 as a quick patch release (and extract it into a proper new rule with the next major/minor release), since this issue seems to affect a lot of people.

[ota-meshi]

The vue/no-setup-props-destructure rule warns of statements that lose reactivity of props values.
https://eslint.vuejs.org/rules/no-setup-props-destructure.html#rule-details
So I think the current rule is working correctly.
You can turn off the rule if you intentionally want to lose props reactivity.
If people don't find this rule useful, I think we can consider removing it from the shareable configuration provided by this plugin.
Also, if the sharable configuration provided by this plugin is not useful for people, we may consider deprecating the sharable configuration provided by the plugin in the next version.

[jacekkarczmarczyk]

I'm not personally against the new behaviour, but the name of the rule doesn't match this behaviour

[daniluk4000]

The vue/no-setup-props-destructure rule warns of statements that lose reactivity of props values.

I'm okay with it's warning on top-level or in arguments - that's very dangerous and could lead to errors.

But in above cases we only trying to set default value for refs. We don't really need reactivity here. My suggestion above was to cover this with an additional options, others thought about complete new rule.

Complete disabling this rule for this new added case could also lead to many unwanted developer errors. We need the rule to cover specific places and not to cover ref assigment (at least with possibility to disable other rule / disable this behavior via some option).

[l3d00m]

The vue/no-setup-props-destructure rule warns of statements that lose reactivity of props values.

Sorry, but this is not true. Your links explicitly states that the rule "reports the destructuring of props passed to setup causing the value to lose reactivity".

And my complaint was specifically about the newly added access linting.

You can turn off the rule if you intentionally want to lose props reactivity.

No, because this rule is incredibly useful! Erroring on destructuring is important for me. Just the new changes are not.

Also, if the sharable configuration provided by this plugin is not useful for people, we may consider deprecating the sharable configuration provided by the plugin in the next version.

I don't understand where you got that from. The only fact so many people are complaining here is because probably everyone uses the shareable config because they are incredibly helpful. I really do appreciate them.

Please understand that I'm not coming from a hostile position. I ask you to reconsider splitting the rules.

The original rule is important and it makes sense to have it as an error rule in essential. The new changes though are subjective and should probably just emit a warning. And they most certainly shouldn't be placed in essential as an error. As a user there's currently no way but to disable the rules for both use cases.

[kleinfreund]

While I understand the frustration this brings with it, the rule as it used to work is incomplete in my opinion. Sure, the name is misleading now, but both destructuring of props and initialization of new refs using props via property access break in the same way (i.e. with regards to reactivity being lost):

const newRef1 = ref(props.test)

const { test } = props
const newRef2 = ref(test)

Neither of the created refs are reactive based on props.test.

There is a simple solution for making a reactive ref out of a prop: toRef

const newRef3 = toRef(() => props.test)

---

If (and multiple people point this out here) the risk of unintentional loss of reactivity through destructuring of props is real and should lead to an error, the same should be true for other ways the same issue can occur. The changed rule addresses this.

[daniluk4000]

There is a simple solution though: toRef

toRef will create readonly ref from my prop, which is not the case. I'll share the example.

const localValue = ref(props.modelValue);
const { modelValue } = props;

const handleValueUpdate = (value: string) => {
    emit('update:modelValue', value);
}

 // Watch and set actual value here
watch(() => props.modelValue, () => localValue.value = props.modelValue);

// Oops! Value remains old and will never change since we used destructure
watch(() => modelValue, () => localValue.value = modelValue);

As you can see in this example, destructuring of props would break my watch as well as anything else. Smart use of top-level assigment would not lead to this thought. What will be your solution for this case?

Also, let's take a look on OAPI realization.

<script lang="ts">
import { defineComponent } from 'vue';

export default defineComponent({
     props: {
          modelValue: {
              type: String,
              required: true,
          },
     },
     data() {
          return {
               localValue: this.modelValue,
          },
     },
     watch: {
         modelValue() {
              this.localValue = this.modelValue;
         },
     },
     methods: {
          handleValueUpdate(value: string) {
               this.$emit('update:modelValue', value);
          },
     },
});
</script>

This is one of classic cases when we need to set default value for our changeable data.

[kleinfreund]

toRef will create readonly ref from my prop

It's intentional that props aren't allowed to be mutated, hence toRef(() => props.test) won't allow you to modify props.test.

---

Looking at this more, it seems to at least be hard to replicate some very common uses cases while leaving the rule active:

const newRef = ref(props.test)

This results in an error as many people pointed out.

But how can one declare that they're fine with losing the reactivity in this example?

const newRef = toRef(props.test)

This works, but you can't mutate newRef. What if you want a local piece of reactive state that you can mutate that's initialized with a prop value?

const propValue = toValue(props.test)
const newRef = ref(propValue)

Unfortunately, the call to toValue(props.test) produces the rule error.

Annoying workaround: Move the props access out of the top-level of the component script:

const newRef = ref(getDefaultValue())

function getDefaultValue () {
  return props.test
}

[davidmyersdev]

I'd like to point out that vue/no-ref-object-destructure actually behaves like this rules does after these changes. In fact, I based my PR on that rule after I noticed the disparity between them. That said, I also think there is confusion here with the meaning of destructuring in this context. Take the following example:

const foo = { bar: 'baz' }
const { bar } = foo

It could just as easily be rewritten as this:

const foo = { bar: 'baz' }
const bar = foo.bar

No, the latter example is not a destructuring assignment, but it accomplishes the same thing. It destructures an object's property accesses into distinct references to the underlying objects or values, which breaks the ability to track said accesses (hence why it breaks reactivity). In fact, if you look at the tests from before I modified the rule, they explicitly test that the latter example produces an error. Preventing the accidental breakage of reactivity is likely why you're using this rule, otherwise disabling it is a simple option here.

That said, yes, it makes sense to support cases where you're making a conscious decision to not keep the reactivity of props (or at least handle it differently), so let's dig into what you can do right now without a change to this rule.

But how can one declare that they're fine with losing the reactivity in this example?

const newRef = toRef(props.test)

While some have pointed out the toRef example, the main complaint is that using it will cause you to lose the ability to update the underly props value. I'd like to recommend you don't do that, as the Vue docs clearly state that props flow one way, but I know that's not always practical in a pre-existing codebase. Instead, I'd recommend you look at one of the overloads of toRef that allows you to maintain that 2-way data flow:

// Specify the prop as the second argument
const newRef = toRef(props, 'test')

My goal in making this change was not to ruin anyone's day. It was actually quite the opposite. As someone who loves Vue, I like to do my part to teach others about how it works, and a big part of that is the reactivity system. When beginners come to Vue and they don't fully understand how it works or they come from frameworks like React (where things are automatically reactive, albeit at the cost of performance), it's not difficult for them to break reactivity. When that happens, especially in a professional setting, it can be really discouraging for them. The goal here is to help catch errors like these and teach others in the process so that they can feel empowered to work in a Vue codebase with less risk.