rancher / dashboard

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

Feature Flags: Computed property "rows" was assigned to but it has no setter. #11175

Closed rak-phillip closed 2 months ago

rak-phillip commented 3 months ago

Dashboard throws a Vue warning when modifying a feature flag that requires a restart:

[Vue warn]: Computed property "rows" was assigned to but it has no setter.

found in

---> <Management.cattle.io.feature> at shell/list/management.cattle.io.feature.vue
       <ResourceList> at shell/components/ResourceList/index.vue
         <ClusterResourcedList> at shell/pages/c/_cluster/_product/_resource/index.vue
           <Shell/components/templates/default.vue> at shell/components/templates/default.vue
             <Root>

This happens because rows is a computed property:

https://github.com/rancher/dashboard/blob/5efe60ef8b932c8fbbfe1f9b613b0a17db40e5be/shell/mixins/resource-fetch.js#L64-L72

but we attempt to reassign rows when the server comes back online:

https://github.com/rancher/dashboard/blob/5efe60ef8b932c8fbbfe1f9b613b0a17db40e5be/shell/list/management.cattle.io.feature.vue#L165-L170

nwmac commented 3 months ago

@richard-cox I had a quick look at the code - I think the fix is to remove the assignment to rows - that line kicks off a fresh fetch of the resources, so the rows computed should pick that up, right?

richard-cox commented 3 months ago

so the rows computed should pick that up, right

Correct, rows should equate to a getter which looks at the store. Changes to that should trigger an update to filteredRows which feeds into the list component. Worth double checking though

rak-phillip commented 3 months ago

Alternatively, we can consider adding a setter^1 to rows if we are concerned about a change in behavior when removing the rows assignment.

eva-vashkevich commented 2 months ago

No unexpected warning or errors are shown. https://github.com/rancher/dashboard/assets/133266076/befd1176-d6e5-4989-b7dd-16e58ddb42a6