Skip to content

Commit 716b831

Browse files
CatchABusNathanWalker
CatchABus
authored and
NathanWalker
committed
feat: binding expression parser additions and improvements (#9791)
1 parent 2efcdf5 commit 716b831

2 files changed

Lines changed: 77 additions & 31 deletions

File tree

‎packages/core/ui/core/bindable/bindable-expressions.ts‎

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ interface ASTExpression {
88

99
const expressionsCache = {};
1010

11+
const FORCED_CHAIN_VALUE = Symbol('forcedChain');
12+
1113
// prettier-ignore
1214
const unaryOperators = {
1315
'+': (v) => +v,
@@ -60,45 +62,55 @@ const expressionParsers = {
6062
}
6163

6264
const left = convertExpressionToValue(expression.left, model, isBackConvert, changedModel);
65+
66+
if (expression.operator == '|' && expression.right.type == 'CallExpression') {
67+
expression.right.requiresConverter = true;
68+
}
6369
const right = convertExpressionToValue(expression.right, model, isBackConvert, changedModel);
6470

6571
if (expression.operator == '|') {
66-
if (right != null && isFunction(right.callback) && right.context != null && right.args != null) {
72+
if (expression.right.requiresConverter && right != null) {
6773
right.args.unshift(left);
6874
return right.callback.apply(right.context, right.args);
6975
}
7076
throw new Error('Invalid converter after ' + expression.operator + ' operator');
7177
}
72-
7378
return binaryOperators[expression.operator](left, right);
7479
},
7580
'CallExpression': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => {
76-
let object;
77-
let property;
78-
if (expression.callee.type == 'MemberExpression') {
79-
object = convertExpressionToValue(expression.callee.object, model, isBackConvert, changedModel);
80-
property = expression.callee.computed ? convertExpressionToValue(expression.callee.property, model, isBackConvert, changedModel) : expression.callee.property?.name;
81+
expression.callee.requiresObjectAndProperty = true;
82+
83+
const { object, property } = convertExpressionToValue(expression.callee, model, isBackConvert, changedModel);
84+
85+
let callback;
86+
if (object == FORCED_CHAIN_VALUE) {
87+
callback = undefined;
8188
} else {
82-
object = getContext(expression.callee.name, model, changedModel);
83-
property = expression.callee?.name;
89+
callback = expression.callee.optional ? object?.[property] : object[property];
8490
}
8591

86-
const callback = expression.callee.optional ? object?.[property] : object[property];
87-
if (isNullOrUndefined(callback)) {
92+
if ((!expression.optional || expression.requiresConverter) && isNullOrUndefined(callback)) {
8893
throw new Error('Cannot perform a call using a null or undefined property');
8994
}
9095

91-
const isConverter = isObject(callback) && (isFunction(callback.toModel) || isFunction(callback.toView));
92-
if (!isFunction(callback) && !isConverter) {
93-
throw new Error('Cannot perform a call using a non-callable property');
96+
if (expression.requiresConverter) {
97+
if (isFunction(callback)) {
98+
callback = { toView: callback };
99+
} else if (!isObject(callback) || !isFunction(callback.toModel) && !isFunction(callback.toView)) {
100+
throw new Error('Invalid converter call');
101+
}
94102
}
95103

96104
const parsedArgs = [];
97105
for (let argument of expression.arguments) {
98106
let value = convertExpressionToValue(argument, model, isBackConvert, changedModel);
99107
argument.type == 'SpreadElement' ? parsedArgs.push(...value) : parsedArgs.push(value);
100108
}
101-
return isConverter ? getConverter(callback, object, parsedArgs, isBackConvert) : callback.apply(object, parsedArgs);
109+
110+
if (expression.requiresConverter) {
111+
return getConverter(callback, object, parsedArgs, isBackConvert);
112+
}
113+
return expression.optional ? callback?.apply(object, parsedArgs) : callback.apply(object, parsedArgs);
102114
},
103115
'ChainExpression': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => {
104116
return convertExpressionToValue(expression.expression, model, isBackConvert, changedModel);
@@ -109,6 +121,9 @@ const expressionParsers = {
109121
},
110122
'Identifier': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => {
111123
const context = getContext(expression.name, model, changedModel);
124+
if (expression.requiresObjectAndProperty) {
125+
return { object: context, property: expression.name };
126+
}
112127
return context[expression.name];
113128
},
114129
'Literal': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => {
@@ -122,8 +137,34 @@ const expressionParsers = {
122137
return logicalOperators[expression.operator](left, () => convertExpressionToValue(expression.right, model, isBackConvert, changedModel));
123138
},
124139
'MemberExpression': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => {
140+
if (expression.object.type == 'MemberExpression') {
141+
expression.object.isChained = true;
142+
}
143+
125144
const object = convertExpressionToValue(expression.object, model, isBackConvert, changedModel);
126145
const property = expression.computed ? convertExpressionToValue(expression.property, model, isBackConvert, changedModel) : expression.property?.name;
146+
const propertyInfo = { object, property };
147+
148+
if (expression.requiresObjectAndProperty) {
149+
return propertyInfo;
150+
}
151+
152+
/**
153+
* If first member is undefined, make sure that no error is thrown later but return undefined instead.
154+
* This behaviour is kept in order to cope with components whose binding context takes a bit long to load.
155+
* Old parser would return undefined for an expression like 'property1.property2.property3'
156+
* even if expression as a whole consisted of undefined properties.
157+
* The new one will keep the same principle only if first member is undefined for safety reasons.
158+
* It meddles with members specifically, so that it will not affect expression result as a whole.
159+
* For example, an 'isLoading || isBusy' expression will be validated as 'undefined || undefined'
160+
* if context is not ready.
161+
*/
162+
if (object === undefined && expression.object.type == 'Identifier') {
163+
return expression.isChained ? FORCED_CHAIN_VALUE : object;
164+
}
165+
if (object == FORCED_CHAIN_VALUE) {
166+
return expression.isChained ? object : undefined;
167+
}
127168
return expression.optional ? object?.[property] : object[property];
128169
},
129170
'NewExpression': (expression: ASTExpression, model, isBackConvert: boolean, changedModel) => {
@@ -211,5 +252,8 @@ export function parseExpression(expressionText: string): ASTExpression {
211252
}
212253

213254
export function convertExpressionToValue(expression: ASTExpression, model, isBackConvert: boolean, changedModel) {
255+
if (!(expression.type in expressionParsers)) {
256+
throw Error('Invalid expression syntax');
257+
}
214258
return expressionParsers[expression.type](expression, model, isBackConvert, changedModel);
215259
}

‎packages/core/ui/core/bindable/index.ts‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -345,22 +345,27 @@ export class Binding {
345345
if (__UI_USE_EXTERNAL_RENDERER__) {
346346
} else if (this.options.expression) {
347347
const changedModel = {};
348-
changedModel[bc.bindingValueKey] = value;
349-
changedModel[bc.newPropertyValueKey] = value;
348+
const targetInstance = this.target.get();
350349
let sourcePropertyName = '';
351350
if (this.sourceOptions) {
352351
sourcePropertyName = this.sourceOptions.property;
353352
} else if (typeof this.options.sourceProperty === 'string' && this.options.sourceProperty.indexOf('.') === -1) {
354353
sourcePropertyName = this.options.sourceProperty;
355354
}
356355

356+
const updateExpression = this.prepareExpressionForUpdate();
357+
this.prepareContextForExpression(targetInstance, changedModel, updateExpression);
358+
359+
/**
360+
* Wait for 'prepareContextForExpression' to assign keys first and override any possible occurences.
361+
* For example, 'bindingValueKey' key can result in a circular reference if it's set in both cases.
362+
*/
363+
changedModel[bc.bindingValueKey] = value;
364+
changedModel[bc.newPropertyValueKey] = value;
357365
if (sourcePropertyName !== '') {
358366
changedModel[sourcePropertyName] = value;
359367
}
360368

361-
const updateExpression = this.prepareExpressionForUpdate();
362-
this.prepareContextForExpression(changedModel, updateExpression);
363-
364369
const expressionValue = this._getExpressionValue(updateExpression, true, changedModel);
365370
if (expressionValue instanceof Error) {
366371
Trace.write((<Error>expressionValue).message, Trace.categories.Binding, Trace.messageType.error);
@@ -373,17 +378,18 @@ export class Binding {
373378
}
374379

375380
private _getExpressionValue(expression: string, isBackConvert: boolean, changedModel: any): any {
376-
let result: any = '';
381+
let result: any = null;
377382

378383
if (!__UI_USE_EXTERNAL_RENDERER__) {
379384
let context;
385+
const targetInstance = this.target.get();
380386
const addedProps = [];
381387
try {
382388
let exp;
383389
try {
384390
exp = parseExpression(expression);
385391
} catch (e) {
386-
result = e;
392+
result = new Error(e + ' at ' + targetInstance);
387393
}
388394

389395
if (exp) {
@@ -397,17 +403,15 @@ export class Binding {
397403
}
398404

399405
// For expressions, there are also cases when binding must be updated after component is loaded (e.g. ListView)
400-
if (this.prepareContextForExpression(context, expression, addedProps)) {
406+
if (this.prepareContextForExpression(targetInstance, context, expression, addedProps)) {
401407
result = convertExpressionToValue(exp, context, isBackConvert, changedModel ? changedModel : context);
402408
} else {
403-
const targetInstance = this.target.get();
404409
targetInstance.off('loaded', this.loadedHandlerVisualTreeBinding, this);
405410
targetInstance.on('loaded', this.loadedHandlerVisualTreeBinding, this);
406411
}
407412
}
408413
} catch (e) {
409-
const errorMessage = 'Run-time error occured in file: ' + e.sourceURL + ' at line: ' + e.line + ' and column: ' + e.column;
410-
result = new Error(errorMessage);
414+
result = new Error(e + ' at ' + targetInstance);
411415
}
412416

413417
// Clear added props
@@ -480,9 +484,7 @@ export class Binding {
480484
}
481485
}
482486

483-
private prepareContextForExpression(model: Object, expression: string, addedProps = []) {
484-
const targetInstance = this.target.get();
485-
487+
private prepareContextForExpression(target: any, model: Object, expression: string, addedProps = []) {
486488
let success = true;
487489
let parentViewAndIndex: { view: ViewBase; index: number };
488490
let parentView;
@@ -497,7 +499,7 @@ export class Binding {
497499
for (let i = 0; i < parentsArray.length; i++) {
498500
// This prevents later checks to mistake $parents[] for $parent
499501
expressionCP = expressionCP.replace(parentsArray[i], '');
500-
parentViewAndIndex = this.getParentView(targetInstance, parentsArray[i]);
502+
parentViewAndIndex = this.getParentView(target, parentsArray[i]);
501503
if (parentViewAndIndex.view) {
502504
model[bc.parentsValueKey] = model[bc.parentsValueKey] || {};
503505
model[bc.parentsValueKey][parentViewAndIndex.index] = parentViewAndIndex.view.bindingContext;
@@ -509,7 +511,7 @@ export class Binding {
509511
}
510512

511513
if (expressionCP.indexOf(bc.parentValueKey) > -1) {
512-
parentView = this.getParentView(targetInstance, bc.parentValueKey).view;
514+
parentView = this.getParentView(target, bc.parentValueKey).view;
513515
if (parentView) {
514516
model[bc.parentValueKey] = parentView.bindingContext;
515517
addedProps.push(bc.parentValueKey);

0 commit comments

Comments
 (0)