From 716b831523829dfa508d32efe0067060109ca574 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 21 Feb 2022 23:09:07 +0200 Subject: [PATCH] feat: binding expression parser additions and improvements (#9791) --- .../ui/core/bindable/bindable-expressions.ts | 74 +++++++++++++++---- packages/core/ui/core/bindable/index.ts | 34 +++++---- 2 files changed, 77 insertions(+), 31 deletions(-) diff --git a/packages/core/ui/core/bindable/bindable-expressions.ts b/packages/core/ui/core/bindable/bindable-expressions.ts index c1450e05d..dee9f0591 100644 --- a/packages/core/ui/core/bindable/bindable-expressions.ts +++ b/packages/core/ui/core/bindable/bindable-expressions.ts @@ -8,6 +8,8 @@ interface ASTExpression { const expressionsCache = {}; +const FORCED_CHAIN_VALUE = Symbol('forcedChain'); + // prettier-ignore const unaryOperators = { '+': (v) => +v, @@ -60,37 +62,43 @@ const expressionParsers = { } const left = convertExpressionToValue(expression.left, model, isBackConvert, changedModel); + + if (expression.operator == '|' && expression.right.type == 'CallExpression') { + expression.right.requiresConverter = true; + } const right = convertExpressionToValue(expression.right, model, isBackConvert, changedModel); if (expression.operator == '|') { - if (right != null && isFunction(right.callback) && right.context != null && right.args != null) { + if (expression.right.requiresConverter && right != null) { right.args.unshift(left); return right.callback.apply(right.context, right.args); } throw new Error('Invalid converter after ' + expression.operator + ' operator'); } - return binaryOperators[expression.operator](left, right); }, 'CallExpression': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => { - let object; - let property; - if (expression.callee.type == 'MemberExpression') { - object = convertExpressionToValue(expression.callee.object, model, isBackConvert, changedModel); - property = expression.callee.computed ? convertExpressionToValue(expression.callee.property, model, isBackConvert, changedModel) : expression.callee.property?.name; + expression.callee.requiresObjectAndProperty = true; + + const { object, property } = convertExpressionToValue(expression.callee, model, isBackConvert, changedModel); + + let callback; + if (object == FORCED_CHAIN_VALUE) { + callback = undefined; } else { - object = getContext(expression.callee.name, model, changedModel); - property = expression.callee?.name; + callback = expression.callee.optional ? object?.[property] : object[property]; } - const callback = expression.callee.optional ? object?.[property] : object[property]; - if (isNullOrUndefined(callback)) { + if ((!expression.optional || expression.requiresConverter) && isNullOrUndefined(callback)) { throw new Error('Cannot perform a call using a null or undefined property'); } - const isConverter = isObject(callback) && (isFunction(callback.toModel) || isFunction(callback.toView)); - if (!isFunction(callback) && !isConverter) { - throw new Error('Cannot perform a call using a non-callable property'); + if (expression.requiresConverter) { + if (isFunction(callback)) { + callback = { toView: callback }; + } else if (!isObject(callback) || !isFunction(callback.toModel) && !isFunction(callback.toView)) { + throw new Error('Invalid converter call'); + } } const parsedArgs = []; @@ -98,7 +106,11 @@ const expressionParsers = { let value = convertExpressionToValue(argument, model, isBackConvert, changedModel); argument.type == 'SpreadElement' ? parsedArgs.push(...value) : parsedArgs.push(value); } - return isConverter ? getConverter(callback, object, parsedArgs, isBackConvert) : callback.apply(object, parsedArgs); + + if (expression.requiresConverter) { + return getConverter(callback, object, parsedArgs, isBackConvert); + } + return expression.optional ? callback?.apply(object, parsedArgs) : callback.apply(object, parsedArgs); }, 'ChainExpression': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => { return convertExpressionToValue(expression.expression, model, isBackConvert, changedModel); @@ -109,6 +121,9 @@ const expressionParsers = { }, 'Identifier': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => { const context = getContext(expression.name, model, changedModel); + if (expression.requiresObjectAndProperty) { + return { object: context, property: expression.name }; + } return context[expression.name]; }, 'Literal': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => { @@ -122,8 +137,34 @@ const expressionParsers = { return logicalOperators[expression.operator](left, () => convertExpressionToValue(expression.right, model, isBackConvert, changedModel)); }, 'MemberExpression': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => { + if (expression.object.type == 'MemberExpression') { + expression.object.isChained = true; + } + const object = convertExpressionToValue(expression.object, model, isBackConvert, changedModel); const property = expression.computed ? convertExpressionToValue(expression.property, model, isBackConvert, changedModel) : expression.property?.name; + const propertyInfo = { object, property }; + + if (expression.requiresObjectAndProperty) { + return propertyInfo; + } + + /** + * If first member is undefined, make sure that no error is thrown later but return undefined instead. + * This behaviour is kept in order to cope with components whose binding context takes a bit long to load. + * Old parser would return undefined for an expression like 'property1.property2.property3' + * even if expression as a whole consisted of undefined properties. + * The new one will keep the same principle only if first member is undefined for safety reasons. + * It meddles with members specifically, so that it will not affect expression result as a whole. + * For example, an 'isLoading || isBusy' expression will be validated as 'undefined || undefined' + * if context is not ready. + */ + if (object === undefined && expression.object.type == 'Identifier') { + return expression.isChained ? FORCED_CHAIN_VALUE : object; + } + if (object == FORCED_CHAIN_VALUE) { + return expression.isChained ? object : undefined; + } return expression.optional ? object?.[property] : object[property]; }, 'NewExpression': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => { @@ -211,5 +252,8 @@ export function parseExpression(expressionText: string): ASTExpression { } export function convertExpressionToValue(expression: ASTExpression, model, isBackConvert: boolean, changedModel) { + if (!(expression.type in expressionParsers)) { + throw Error('Invalid expression syntax'); + } return expressionParsers[expression.type](expression, model, isBackConvert, changedModel); } diff --git a/packages/core/ui/core/bindable/index.ts b/packages/core/ui/core/bindable/index.ts index 1e22e0458..41b268745 100644 --- a/packages/core/ui/core/bindable/index.ts +++ b/packages/core/ui/core/bindable/index.ts @@ -345,8 +345,7 @@ export class Binding { if (__UI_USE_EXTERNAL_RENDERER__) { } else if (this.options.expression) { const changedModel = {}; - changedModel[bc.bindingValueKey] = value; - changedModel[bc.newPropertyValueKey] = value; + const targetInstance = this.target.get(); let sourcePropertyName = ''; if (this.sourceOptions) { sourcePropertyName = this.sourceOptions.property; @@ -354,13 +353,19 @@ export class Binding { sourcePropertyName = this.options.sourceProperty; } + const updateExpression = this.prepareExpressionForUpdate(); + this.prepareContextForExpression(targetInstance, changedModel, updateExpression); + + /** + * Wait for 'prepareContextForExpression' to assign keys first and override any possible occurences. + * For example, 'bindingValueKey' key can result in a circular reference if it's set in both cases. + */ + changedModel[bc.bindingValueKey] = value; + changedModel[bc.newPropertyValueKey] = value; if (sourcePropertyName !== '') { changedModel[sourcePropertyName] = value; } - const updateExpression = this.prepareExpressionForUpdate(); - this.prepareContextForExpression(changedModel, updateExpression); - const expressionValue = this._getExpressionValue(updateExpression, true, changedModel); if (expressionValue instanceof Error) { Trace.write((expressionValue).message, Trace.categories.Binding, Trace.messageType.error); @@ -373,17 +378,18 @@ export class Binding { } private _getExpressionValue(expression: string, isBackConvert: boolean, changedModel: any): any { - let result: any = ''; + let result: any = null; if (!__UI_USE_EXTERNAL_RENDERER__) { let context; + const targetInstance = this.target.get(); const addedProps = []; try { let exp; try { exp = parseExpression(expression); } catch (e) { - result = e; + result = new Error(e + ' at ' + targetInstance); } if (exp) { @@ -397,17 +403,15 @@ export class Binding { } // For expressions, there are also cases when binding must be updated after component is loaded (e.g. ListView) - if (this.prepareContextForExpression(context, expression, addedProps)) { + if (this.prepareContextForExpression(targetInstance, context, expression, addedProps)) { result = convertExpressionToValue(exp, context, isBackConvert, changedModel ? changedModel : context); } else { - const targetInstance = this.target.get(); targetInstance.off('loaded', this.loadedHandlerVisualTreeBinding, this); targetInstance.on('loaded', this.loadedHandlerVisualTreeBinding, this); } } } catch (e) { - const errorMessage = 'Run-time error occured in file: ' + e.sourceURL + ' at line: ' + e.line + ' and column: ' + e.column; - result = new Error(errorMessage); + result = new Error(e + ' at ' + targetInstance); } // Clear added props @@ -480,9 +484,7 @@ export class Binding { } } - private prepareContextForExpression(model: Object, expression: string, addedProps = []) { - const targetInstance = this.target.get(); - + private prepareContextForExpression(target: any, model: Object, expression: string, addedProps = []) { let success = true; let parentViewAndIndex: { view: ViewBase; index: number }; let parentView; @@ -497,7 +499,7 @@ export class Binding { for (let i = 0; i < parentsArray.length; i++) { // This prevents later checks to mistake $parents[] for $parent expressionCP = expressionCP.replace(parentsArray[i], ''); - parentViewAndIndex = this.getParentView(targetInstance, parentsArray[i]); + parentViewAndIndex = this.getParentView(target, parentsArray[i]); if (parentViewAndIndex.view) { model[bc.parentsValueKey] = model[bc.parentsValueKey] || {}; model[bc.parentsValueKey][parentViewAndIndex.index] = parentViewAndIndex.view.bindingContext; @@ -509,7 +511,7 @@ export class Binding { } if (expressionCP.indexOf(bc.parentValueKey) > -1) { - parentView = this.getParentView(targetInstance, bc.parentValueKey).view; + parentView = this.getParentView(target, bc.parentValueKey).view; if (parentView) { model[bc.parentValueKey] = parentView.bindingContext; addedProps.push(bc.parentValueKey);