fix(compiler): don't report parse error for interpolation inside string in property binding #40267
Conversation
…ng in property binding
Currently we check whether a property binding contains an interpolation using a regex so
that we can throw an error. The problem is that the regex doesn't account for quotes
which means that something like `[prop]="'{{ foo }}'"` will be considered an error, even
though it's not actually an interpolation.
These changes build on top of the logic from angular#39826 to account for interpolation
characters inside quotes.
Fixes angular#39601.
pullapprove
bot
added
area: compiler
crisbeto
added
action: review
| this._forEachUnquotedChar(input, 0, charIndex => { | ||
| if (startIndex === -1) { | ||
| startIndex = input.indexOf(start, charIndex); | ||
| return false; | ||
| } else { | ||
| endIndex = this._getInterpolationEndIndex(input, end, charIndex); | ||
| return endIndex > -1; | ||
| } | ||
| }); |
There was a problem hiding this comment.
I think that this is a bit inefficient, since it will continue to run through each unquoted character even indexOf found the start.
Also I am concerned that the following string would find a false positive:
abc '{{' }} def
since the {{ is in a string, it should be ignored but would be picked up by the indexOf(), when the charIndex is 0.
There was a problem hiding this comment.
I think this would also be clearer if you created a new function _getInterpolationStartIndex(input, expressionStart, start):
private _getInterpolationStartIndex(input: string, expressionStart: string, start: number): number {
let result = -1;
this._forEachUnquotedChar(input, start, charIndex => {
if (input.startsWith(expressionStart, charIndex)) {
result = charIndex;
return true;
} else {
return false;
}
});
return result;Then you could do:
const startIndex = this._getInterpolationStartIndex(input, start, 0);
if (startIndex === -1) {
return;
}
const endIndex = this._getInterpolationEndIndex(input, end, startIndex+start.length);
if (endIndex === -1) {
return;
}
this._reportError(...);There was a problem hiding this comment.
I didn't want to add a _getInterpolationStartIndex, because there's some other logic that looks for an interpolation start in splitInterpolation, but the requirements there are slightly different. As for the abc '{{' }} def case, I think I tried something similar and it was reported as a different (syntax?) error further down the compilation process.
…de string in property binding
|
I've added the extra test cases and addressed the feedback. @petebacondarwin |
…de string in property binding
…de string in property binding
crisbeto
added
merge: caretaker note
|
FYI, presubmit is successful for the changes in this PR. |
AndrewKushnir
removed
the
action: presubmit
AndrewKushnir
removed
the
merge: caretaker note
…ng in property binding (#40267) Currently we check whether a property binding contains an interpolation using a regex so that we can throw an error. The problem is that the regex doesn't account for quotes which means that something like `[prop]="'{{ foo }}'"` will be considered an error, even though it's not actually an interpolation. These changes build on top of the logic from #39826 to account for interpolation characters inside quotes. Fixes #39601. PR Close #40267
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently we check whether a property binding contains an interpolation using a regex so that we can throw an error. The problem is that the regex doesn't account for quotes which means that something like
[prop]="'{{ foo }}'"will be considered an error, even though it's not actually an interpolation.These changes build on top of the logic from #39826 to account for interpolation characters inside quotes.
Fixes #39601.