diff --git a/resources/js/components/field-conditions/Validator.js b/resources/js/components/field-conditions/Validator.js index 1183a12e81e..d4fe97ce5f4 100644 --- a/resources/js/components/field-conditions/Validator.js +++ b/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,6 +59,21 @@ 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) { @@ -63,18 +81,21 @@ export default class { } 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; diff --git a/resources/js/tests/FieldConditionsValidator.test.js b/resources/js/tests/FieldConditionsValidator.test.js index 742c84dca0c..59b537d0429 100644 --- a/resources/js/tests/FieldConditionsValidator.test.js +++ b/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); +});