-
Notifications
You must be signed in to change notification settings - Fork 27k
feat(core): support regular expressions in templates #63857
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Updates the expression lexer to produce tokens for regular expression literals.
Updates the template syntax to support inline regular expressions.
Adds the `RegularExpressionLiteral` node to the output AST.
…mpiler Handles regular expression literals across the template type checker and the various translators.
Adds support for regular expression literals in the template pipeline and adds some logic to optimize non-global ones.
Updates the docs to mention that regular expressions are supported.
devversion
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
| !lastToken.isString() && | ||
| !lastToken.isKeyword() && | ||
| !lastToken.isCharacter(chars.$RPAREN) && | ||
| !lastToken.isCharacter(chars.$RBRACKET) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what places is a regex starting? would it be less cases to catch where it can actually follow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this logic, because the lexer works on a character-by-character basis. When we see a slash, we don't know if it'll be a regex or something like a / b.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I guess I was trying to ask if it would be simpler to inverse the checks here. E.g. check for left parenthesis— but I think it would likely be more verbose than this, so LGTM
| } | ||
|
|
||
| override isConstant() { | ||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this have an affect on the template pipeline? do you recall where this is used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly, we use it together with the constant pool to determine if a value can be extracted into a variable and reused. I'd consider regexes to be constant since they're immutable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I just wondered if the optimization logic with the g flag could be handled via isConstant here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried out removing the explicit optimization and relying on isConstant and the ConstantPool doing the right thing and it didn't work so things might've stopped working properly there at some point...
|
Caretaker note: we may have to patch cl/808162868 into the sync CL. |
Updates the template syntax to support inline regular expressions. PR Close #63857
Adds the `RegularExpressionLiteral` node to the output AST. PR Close #63857
|
This PR was merged into the repository. The changes were merged into the following branches:
|
Updates the docs to mention that regular expressions are supported. PR Close #63857
…ngular#63857)" This reverts commit 32c98e0.
…o pipeline (angular#63857)" This reverts commit dd77233.
…n AOT compiler (angular#63857)" This reverts commit f2ef838.
…ns (angular#63857)" This reverts commit a1288f8.
…ular#63857)" This reverts commit 8a69c06.
Updates the expression lexer to produce tokens for regular expression literals. PR Close angular#63857
Updates the template syntax to support inline regular expressions. PR Close angular#63857
…lar#63857) Adds the `RegularExpressionLiteral` node to the output AST. PR Close angular#63857
…mpiler (angular#63857) Handles regular expression literals across the template type checker and the various translators. PR Close angular#63857
…ne (angular#63857) Adds support for regular expression literals in the template pipeline and adds some logic to optimize non-global ones. PR Close angular#63857
…#63857) Updates the docs to mention that regular expressions are supported. PR Close angular#63857
…ngular#63857)" (angular#63883) This reverts commit 32c98e0. PR Close angular#63883
…o pipeline (angular#63857)" (angular#63883) This reverts commit dd77233. PR Close angular#63883
…n AOT compiler (angular#63857)" (angular#63883) This reverts commit f2ef838. PR Close angular#63883
…ns (angular#63857)" (angular#63883) This reverts commit a1288f8. PR Close angular#63883
…#63857)" (angular#63883) This reverts commit 328a2bf. PR Close angular#63883
…ular#63857)" (angular#63883) This reverts commit 8a69c06. PR Close angular#63883
|
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. |
Updates the template syntax to support inline regular expressions. Also includes some logic to reuse regular expressions between call sites, unless they're marked as global.