fix: update HTML attributes list #3018
Conversation
| const GLOBAL_ATTRIBUTE = new Set([ | ||
| 'role', |
There was a problem hiding this comment.
I removed role because it is not in MDN's list of global attributes.
This change should not be observable, because both @lwc/template-compiler and @lwc/engine-server were also checking isAriaAttribute, and it turns out that role is an ARIA attribute.
lwc/packages/@lwc/template-compiler/src/parser/attribute.ts
Lines 219 to 220 in dba53e7
lwc/packages/@lwc/shared/src/aria.ts
Line 66 in d93e296
| 'class', | ||
| 'contenteditable', | ||
| 'contextmenu', | ||
| 'dir', | ||
| 'draggable', | ||
| 'dropzone', |
There was a problem hiding this comment.
dropzone is not mentioned on MDN. In the HTML5 spec, it is described as an "obsolete" attribute:
dropzoneon all elements
Use script to handle the dragenter and dragover events instead.
Removing it should not have an observable impact, except that users will get a warning whereas they didn't get a warning before.
| return attrName === 'key' || attrName === 'slot'; | ||
| function isFrameworkAttribute(attrName: string): boolean { | ||
| // 'key' is currently the only LWC framework-specific attribute that doesn't start with "lwc:" | ||
| return attrName === 'key'; |
There was a problem hiding this comment.
There is no need to put slot here because it's already in the list of global HTML attributes, and isFrameworkAttribute is only used in one place, which is the isValidHTMLAttribute check, which already checks the list of global HTML attributes.
| @@ -31,10 +31,12 @@ | |||
| */ | |||
| import { hasOwnProperty } from '@lwc/shared'; | |||
|
|
|||
| // This mapping is based on https://github.com/wooorm/html-element-attributes/blob/270d8ce/index.js | |||
There was a problem hiding this comment.
This is where the original list came from, but it was not mentioned anywhere. Probably it was taken from an older version of the library, so I just updated it.
There was a problem hiding this comment.
Side note: I would love to switch to just using the html-element-attributes library directly (as well as svg-element-attributes), but they dropped support for CommonJS, so we can't switch until we fix #3017.
There was a problem hiding this comment.
Opened #3019 to track this.
| 'target', | ||
| 'type', | ||
| ], | ||
| abbr: ['title'], |
There was a problem hiding this comment.
Some of these mappings were dropped, but again, this should only affect warnings.
Details
Fixes #3016
Updates our list of known HTML attributes and known tag-to-attribute mapping, using this list on MDN of global attributes and the latest
html-element-attributescode.Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
There are two observable changes:
@lwc/template-compilerwarnings have changed. There are some new warnings and some dropped warnings.@lwc/engine-serveris also using some of these attributes to determine how to handle props:lwc/packages/@lwc/engine-server/src/renderer.ts
Lines 169 to 172 in fda8d6f
Since the first involves only warnings (not errors), it should not break any existing users.
Since the second is still somewhat experimental, it also shouldn't break anyone due to the observable change.