-
Notifications
You must be signed in to change notification settings - Fork 394
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
feat(template-compiler): enable quoted attribute expressions #4543
Conversation
@@ -109,6 +111,10 @@ export const enum APIFeature { | |||
* If enabled, add support for complex class expressions in the template. | |||
*/ | |||
TEMPLATE_CLASS_NAME_OBJECT_BINDING, |
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.
Just noticed that this isn't ENABLE_
like the rest. Oh well, too late! 🫠
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.
It's not too late; we can change this...
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.
LGTM as a first step, but adding a temporary compiler flag would give me the warm-and-fuzzies.
"start": 22, | ||
"length": 17 | ||
} | ||
}, | ||
{ | ||
"code": 1035, | ||
"message": "LWC1035: Ambiguous attribute value title={myValue}checked. If you want to make it a string you should escape it title=\"\\{myValue}checked\"", |
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.
This error message is not super helpful anymore; it should probably say "go bump your API version" or something... Although I guess that's for later.
if ( | ||
isQuoted && | ||
!isAPIFeatureEnabled(APIFeature.ENABLE_COMPLEX_TEMPLATE_EXPRESSIONS, ctx.apiVersion) | ||
) { |
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'm nervous even just putting this out behind API versioning. Technically it would be a breaking change for off-core consumers or internal consumers if we change it during the release.
I think we should put this behind a (temporary) compiler flag as well.
Details
Introduction of API feature for
ENABLE_COMPLEX_TEMPLATE_EXPRESSIONS
behind v63. Still need to replace all usage ofexperimentalComplexExpressions
but will be doing that incrementally.Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item
W-16348355