fix(template-compiler): relax attribute name restriction on usage of hyphen and underscore in combination @W-12021741 #3143
Conversation
| @@ -1335,7 +1335,8 @@ function applyAttributes(ctx: ParserCtx, parsedAttr: ParsedAttribute, element: B | |||
|
|
|||
| // disallow attr name which combines underscore character with special character. | |||
| // We normalize camel-cased names with underscores caMel -> ca-mel; thus sanitization. | |||
| if (name.match(/_[^a-z0-9]|[^a-z0-9]_/)) { | |||
| // Note 1: underscore followed by a hyphen is valid to accomodate property names like thirsty_Camel -> thirsty_-camel | |||
| if (name.match(/_[^a-z0-9-]|[^a-z0-9]_/)) { | |||
There was a problem hiding this comment.
This is the only code change in this PR, the rest are all tests.
There was a problem hiding this comment.
For a follow-up PR: What is the value of restricting the following pattern /_[^a-z0-9-]|[^a-z0-9]_/? The only remaining test case is under-_hyphen. This name is indeed ambiguous, that said it also blocks other names like foo__bar which is a valid JavaScript identifier.
There was a problem hiding this comment.
What is the value of restricting the following pattern /[^a-z0-9-]|[^a-z0-9]/?
Yeah, that is a great question. I do not see much value in having this restriction IMO. I added the other test cases(valid-non-alphanumeric/actual.html, valid-hyphen-attribute/actual.html) to establish the current treatment of non alpha numeric characters.
If non-alpha numeric characters are allowed in an attribute name in general, why only restrict is in combination with an underscore(_), at the start and end of attribute name? The follow up work is to explore more about these restrictions and take the next steps as part of W-12021709.
tl;dr
Going back into the history of this restriction, I have come to the conclusion that it was added out of a mis-understanding.
- This is the original issue: [compiler] compiler doesn't validate public properties with underscore names #1369. Note that there wasn't an agreement whether allowing underscore in attribute name is a problem.
- The PR that added the restriction(fix(compiler): prevent underscore attr name camelcasing #1385) was meant to prevent camel casing attribute names with underscore.
There was a problem hiding this comment.
Followup PR #3152
| }; | ||
| const stc3 = { | ||
| props: { | ||
| hyphen3pecial: "bar", |
There was a problem hiding this comment.
The AST from L21-44 is to document the existing behavior.
|
/nucleus test |
.../@lwc/template-compiler/src/__tests__/fixtures/attributes/valid-hyphen-attribute/expected.js
Show resolved
Hide resolved
| @@ -1335,7 +1335,8 @@ function applyAttributes(ctx: ParserCtx, parsedAttr: ParsedAttribute, element: B | |||
|
|
|||
| // disallow attr name which combines underscore character with special character. | |||
| // We normalize camel-cased names with underscores caMel -> ca-mel; thus sanitization. | |||
| if (name.match(/_[^a-z0-9]|[^a-z0-9]_/)) { | |||
| // Note 1: underscore followed by a hyphen is valid to accomodate property names like thirsty_Camel -> thirsty_-camel | |||
| if (name.match(/_[^a-z0-9-]|[^a-z0-9]_/)) { | |||
There was a problem hiding this comment.
For a follow-up PR: What is the value of restricting the following pattern /_[^a-z0-9-]|[^a-z0-9]_/? The only remaining test case is under-_hyphen. This name is indeed ambiguous, that said it also blocks other names like foo__bar which is a valid JavaScript identifier.
Details
Relax the restriction on template compiler to allow usage of an underscore(
_) followed by a hyphen(-) in an attribute name.Also add tests to document additional test case for the current restrictions in template compiler.
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item
W-12021741