Skip to content

Commit

Permalink
Update model reactivety
Browse files Browse the repository at this point in the history
- `reactive` was applied to objects that made it into the store via loadAll (or loadPage)
- objects that are assigned to a component `data` object should also be reactive
- however there are scenarios where changes to existing or new properties on these object did not trigger reactivity (computed, watch, etc)
- fix is two parts
  - Ensure all model instances that make it into the store are reactive
    - where we just stick something from `classify` straight into the store... wrap it with reactive
  - Ensure new properties added to a reactive object aren't ignored
    - in `replace` re-reactive the object if there's new properties
  • Loading branch information
richard-cox committed Oct 8, 2024
1 parent 1e8da03 commit 4d56a7f
Showing 1 changed file with 25 additions and 4 deletions.
29 changes: 25 additions & 4 deletions shell/plugins/dashboard-store/mutations.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { markRaw, reactive } from 'vue';
import { markRaw, reactive, toRaw, toRef } from 'vue';

Check warning on line 1 in shell/plugins/dashboard-store/mutations.js

View workflow job for this annotation

GitHub Actions / lint

'toRaw' is defined but never used

Check warning on line 1 in shell/plugins/dashboard-store/mutations.js

View workflow job for this annotation

GitHub Actions / lint

'toRef' is defined but never used
import { addObject, addObjects, clear, removeObject } from '@shell/utils/array';
import { SCHEMA, COUNT } from '@shell/config/types';
import { normalizeType, keyFieldFor } from '@shell/plugins/dashboard-store/normalize';
Expand Down Expand Up @@ -46,15 +46,24 @@ function registerType(state, type) {
}

export function replace(existing, data) {
const keyMap = {};

for ( const k of Object.keys(existing) ) {
delete existing[k];
keyMap[k] = true;
}

let newProperty = false;

for ( const k of Object.keys(data) ) {
if (!newProperty && !keyMap[k]) {
newProperty = true;
}

existing[k] = data[k];
}

return existing;
return newProperty ? reactive(existing) : existing;
}

function replaceResource(existing, data, getters) {
Expand Down Expand Up @@ -125,7 +134,7 @@ export function load(state, {
entry = replaceResource(entry, data, getters);
} else {
// There's no entry, make a new proxy
entry = classify(ctx, data);
entry = reactive(classify(ctx, data));
}
}

Expand Down Expand Up @@ -268,7 +277,7 @@ export function batchChanges(state, { ctx, batch }) {
if (normalizedType === SCHEMA) {
addSchemaIndexFields(resource);
}
const classyResource = classify(ctx, resource);
const classyResource = reactive(classify(ctx, resource));

if (index === undefined) {
typeCache.list.push(classyResource);
Expand Down Expand Up @@ -396,6 +405,9 @@ export default {
}
},

/**
* Load the results of a request that used a selector (like label)
*/
loadSelector(state, {
type, entries, ctx, selector, revision
}) {
Expand All @@ -412,6 +424,9 @@ export default {
cache.revision = revision || 0;
},

/**
* Load the results of a request to fetch all resources or all resources in a namespace
*/
loadAll,

/**
Expand Down Expand Up @@ -441,8 +456,14 @@ export default {
});
},

/**
* Load resources, but don't set `haveAll`
*/
loadAdd,

/**
* Load the results of a request for a page. Often used to exercise advanced filtering
*/
loadPage(state, {
type,
data,
Expand Down

0 comments on commit 4d56a7f

Please sign in to comment.