From 4c7383a93eb35a1cacb193d9bb90eb6913c97fc2 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 24 Aug 2026 16:39:31 -0400 Subject: [PATCH] Memoize resolved field conditions and hoist their regexes getConditions() gets called repeatedly for a single field - twice inside passesNonRevealerConditions() alone - and re-runs the blueprint conversion every time. Resolve it once per validator instead. It isn't a pure getter, though. It also sets passOnAny and showOnPass, so the memoized path has to replay those, and like the uncached path it only ever sets them rather than putting them back to their defaults. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/field-conditions/Validator.js | 31 +++++-- .../js/tests/FieldConditionsValidator.test.js | 84 +++++++++++++++++++ 2 files changed, 110 insertions(+), 5 deletions(-) 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); +});