[6.x] Memoize resolved field conditions

resources/js/components/field-conditions/Validator.js

@@ -5,6 +5,8 @@ import { data_get } from '../../util/data_get.js';
 import { isObject, intersection } from 'lodash-es';
 
 const NUMBER_SPECIFIC_COMPARISONS = ['>', '>=', '<', '<='];
+const CUSTOM_PREFIX_RE = /^custom /;
+const ROOT_PREFIX_RE = /^\$?root\./;
 
 const isEmpty = (value) => {
     if (value === null || value === undefined) return true;
@@ -25,6 +27,7 @@ export default class {
         this.passOnAny = false;
         this.showOnPass = true;
         this.converter = new Converter();
+        this._conditionsResolved = false;
     }
 
     usingRootValues() {
@@ -56,25 +59,43 @@ export default class {
     }
 
     getConditions() {
+        // Memoized per instance. It's not a pure getter — it also sets passOnAny and
+        // showOnPass — so a cached call has to replay them. Like the uncached path, the
+        // replay only ever sets the flags; it never puts them back to their defaults.
+        if (this._conditionsResolved) {
+            if (this._setPassOnAny) this.passOnAny = true;
+            if (this._setShowOffPass) this.showOnPass = false;
+
+            return this._conditions;
+        }
+
+        this._conditionsResolved = true;
+        this._setPassOnAny = false;
+        this._setShowOffPass = false;
+        this._conditions = undefined;
+
         let key = KEYS.filter((key) => this.field[key])[0];
 
         if (!key) {
             return undefined;
         }
 
         if (key.includes('any')) {
-            this.passOnAny = true;
+            this.passOnAny = this._setPassOnAny = true;
         }
 
         if (key.includes('unless') || key.includes('hide_when')) {
             this.showOnPass = false;
+            this._setShowOffPass = true;
         }
 
         let conditions = this.field[key];
 
-        return this.isCustomConditionWithoutTarget(conditions)
+        this._conditions = this.isCustomConditionWithoutTarget(conditions)
             ? conditions
             : this.converter.fromBlueprint(conditions, this.field.prefix);
+
+        return this._conditions;
     }
 
     isCustomConditionWithoutTarget(conditions) {
@@ -189,7 +210,7 @@ export default class {
     }
 
     prepareFunctionName(condition) {
-        return condition.replace(new RegExp('^custom '), '').split(':')[0];
+        return condition.replace(CUSTOM_PREFIX_RE, '').split(':')[0];
     }
 
     prepareParams(condition) {
@@ -204,7 +225,7 @@ export default class {
         }
 
         if (field.startsWith('$root.') || field.startsWith('root.')) {
-            return data_get(this.rootValues, field.replace(new RegExp('^\\$?root\\.'), ''));
+            return data_get(this.rootValues, field.replace(ROOT_PREFIX_RE, ''));
         }
 
         return data_get(this.values, field);
@@ -293,7 +314,7 @@ export default class {
         }
 
         if (lhs.startsWith('$root.') || lhs.startsWith('root.')) {
-            return lhs.replace(new RegExp('^\\$?root\\.'), '');
+            return lhs.replace(ROOT_PREFIX_RE, '');
         }
 
         return dottedPrefix ? dottedPrefix + '.' + lhs : lhs;

resources/js/tests/FieldConditionsValidator.test.js

@@ -5,6 +5,7 @@ import { data_get } from '../bootstrap/globals';
 import FieldConditions from '@/components/FieldConditions';
 import PublishContainer from '@ui/Publish/Container.vue';
 import ShowField from '@/components/field-conditions/ShowField.js';
+import Validator from '@/components/field-conditions/Validator.js';
 
 // Even though there's no Store anymore, this variable is named Store so that all the
 // assertions don't need to be changed. This is now a reference to the PublishContainer component.
@@ -1074,3 +1075,86 @@ test('it can use extra values in conditions', () => {
     expect(showFieldIf({ hello: 'world' })).toBe(true);
     expect(showFieldIf({ hello: 'there' })).toBe(false);
 });
+
+test('it memoizes conditions without losing the flags getConditions() sets', () => {
+    // getConditions() isn't a pure getter. It also sets passOnAny and showOnPass, and it
+    // gets called more than once per validator (passesNonRevealerConditions calls it twice).
+    // Repeat calls must leave the instance in the same state as the first one.
+    const cases = {
+        if: { passOnAny: false, showOnPass: true },
+        if_any: { passOnAny: true, showOnPass: true },
+        show_when: { passOnAny: false, showOnPass: true },
+        show_when_any: { passOnAny: true, showOnPass: true },
+        unless: { passOnAny: false, showOnPass: false },
+        unless_any: { passOnAny: true, showOnPass: false },
+        hide_when: { passOnAny: false, showOnPass: false },
+        hide_when_any: { passOnAny: true, showOnPass: false },
+    };
+
+    Object.entries(cases).forEach(([key, expected]) => {
+        const validator = new Validator({ handle: 'test', [key]: { first_name: 'is Rincess' } }, {}, {}, 'test', [], {});
+
+        const first = validator.getConditions();
+        expect({ passOnAny: validator.passOnAny, showOnPass: validator.showOnPass }).toEqual(expected);
+
+        const second = validator.getConditions();
+        expect({ passOnAny: validator.passOnAny, showOnPass: validator.showOnPass }).toEqual(expected);
+
+        const third = validator.getConditions();
+        expect({ passOnAny: validator.passOnAny, showOnPass: validator.showOnPass }).toEqual(expected);
+
+        expect(second).toEqual(first);
+        expect(third).toEqual(first);
+    });
+});
+
+test('it memoizes a field with no conditions without touching the flags', () => {
+    const validator = new Validator({ handle: 'test' }, {}, {}, 'test', [], {});
+
+    expect(validator.getConditions()).toBe(undefined);
+    expect(validator.getConditions()).toBe(undefined);
+    expect(validator.passOnAny).toBe(false);
+    expect(validator.showOnPass).toBe(true);
+});
+
+test('it gives the same answer whether conditions are evaluated once or twice', () => {
+    // passesConditions() reads passOnAny/showOnPass that getConditions() sets, so a
+    // validator that has already resolved its conditions must still evaluate the same.
+    setValues({ first_name: 'Rincess', last_name: 'Pleia' });
+
+    const configs = [
+        { unless: { first_name: 'is Rincess' } },
+        { if_any: { first_name: 'is Rincess', last_name: 'is Holo' } },
+        { unless_any: { first_name: 'is San', last_name: 'is Holo' } },
+        { hide_when: { first_name: 'is Rincess' } },
+    ];
+
+    configs.forEach((config) => {
+        const validator = new Validator({ handle: 'test', ...config }, Store.values, Store.values, 'test', [], {});
+
+        expect(validator.passesConditions()).toBe(validator.passesConditions());
+    });
+});
+
+test('it does not let evaluation mutate the memoized conditions', () => {
+    // The memoized array is now handed out to every caller, so nothing downstream is
+    // allowed to write to it. passesNonRevealerConditions() in particular filters it and
+    // re-evaluates, and 'empty' comparisons rewrite the condition they're given.
+    setValues({ first_name: 'Rincess', favorite_animals: [] });
+
+    const validator = new Validator(
+        { handle: 'test', if: { first_name: 'is Rincess', favorite_animals: 'is empty' } },
+        Store.values,
+        Store.values,
+        'test',
+        [],
+        {},
+    );
+
+    const before = JSON.parse(JSON.stringify(validator.getConditions()));
+
+    validator.passesConditions();
+    validator.passesNonRevealerConditions('');
+
+    expect(validator.getConditions()).toEqual(before);
+});