rancher / dashboard

The Rancher UI
https://rancher.com
Apache License 2.0
441 stars 241 forks source link

Prevent errors in existing cases binding props with v-model #11029

Open cnotv opened 1 month ago

cnotv commented 1 month ago

Description

Investigate issues printed out by the Vue3 builder about the use of props in v-model.

Context

Since Vue3, it is not possible to rebind props with v-model. Compiler error:

VueCompilerError: v-model cannot be used on a prop, because local prop bindings are not writable.
Use a v-bind binding combined with a v-on listener that emits update:x event instead.

The error is generated from vue/core and there are at the moment 54 occurrences. Specifically to this codebase, it happens mostly when we define in the markup v-model="value", at the moment 86 occurrences.

E.g.:

Screenshot 2024-05-14 at 20 16 19

Please note:

Solution

In the current phase, the only solution seems to replace the prop with a computed property. This can be automated and solved by using a mixin that would add prop modelValue and computed value, then remove the prop value (example PR).

Using existing migration script, we would need to remove all the occurrences where we define value as a prop, including mixins and components in .js format.

References

Collaterals

As in Vue3 the v-model binding is generating a modelValue instead of value, we'll need to apply this mixing everywhere.

cnotv commented 1 month ago

Crossing an issue using the mixin, due to the non-matching multiple cases. Example cases of values props to be replaced:


props: {
  value: {
    type:     Object,
    required: true,
  },
  value: {
    type:    Array,
    default: null,
  },
  value: {
    type:    [Array, Object],
    default: null,
  },
  value: {
    type:    String,
    default: ''
  },
  value: {
    type:    [Array, Object],
    default: null,
  },
  value: {
    type:    Object,
    default: () => {
      return {};
    }
  },
  value: {
    type:    Object as PropType<Badge>,
    default: null
  },
  value: {
    type:    [Boolean, Array, String] as PropType<boolean | boolean[] | string>,
    default: false
  },
  value: {
    required:  true,
    validator: () => true
  },
  value: {
    type:     Number,
    required: true,
    validator(value) {
      return value >= 0;
    }
  },
  value: {
    default: null,
    type:    [String, Object, Number, Array]
  },
}
gaktive commented 1 week ago

Moving to 2.10.0 since we're getting close to feature complete for 2.9.0 and this feels like it can be merged after. However, if this is in a branch, that makes this less risky to merge.