Filter invalid span props #584
Conversation
|
@chawes13 Can I get general feedback on the approach/util before I proceed with adding unit tests and updating other instances of |
| @@ -0,0 +1,54 @@ | |||
| import { htmlElementAttributes } from 'html-element-attributes' | |||
| import htmlAttributes from 'html-attributes' // An object of all HTML attributes with the JSX prop (e.g., "className", "tabIndex") as keys and the JavaScript prop (e.g., "class", "tabindex") as values | |||
There was a problem hiding this comment.
Is the value the "JavaScript prop" or the html attribute?
| import { difference, omitBy, compact, concat } from 'lodash' | ||
| import filterInvalidDOMProps from 'filter-invalid-dom-props' | ||
|
|
||
| // An array of basic HTML _global_ attributes (note: this does not include event handlers, "aria-", "role", and "data-") |
There was a problem hiding this comment.
Maybe provide a link to the global attributes mdn documentation?
| * A function that filters out props 1) that are not recognized as a valid HTML attribute | ||
| * and 2) that are valid HTML attributes but not allowed for the provided element. | ||
| * | ||
| * @param {String} element - HTML element |
There was a problem hiding this comment.
I was a little confused when I read this because HTML element and String seem contradictory. Can we be more explicit by saying something like, "HTML element name"?
| */ | ||
|
|
||
| function filterInvalidHtmlProps(element, props) { | ||
| // This util (filterInvalidDOMProps) covers event handlers such as "onClick" and props that begin with "data-" and "aria-" |
There was a problem hiding this comment.
What does "covers" mean in this context?
| return omitBy(validDOMProps, (_, key) => | ||
| notAllowedAttributes.includes(htmlAttributes[key]) | ||
| ) |
There was a problem hiding this comment.
This should probably be optimized, but I think we should only look at that after benchmarking filterInvalidHtmlProps against filterinvalidDOMProps.
It probably does make sense to write specs first, since then you can change the underlying code under green with confidence.
Release Notes
Author Checklist