-
Notifications
You must be signed in to change notification settings - Fork 1
Filter invalid span props #584
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe provide a link to the global attributes mdn documentation? |
||
const basicGlobalAttributes = htmlElementAttributes['*'] | ||
// An array of all existing HTML attributes (note: this does not include event handlers, "aria-" and "data-") | ||
const allHtmlAttributes = Object.values(htmlAttributes) | ||
|
||
/** | ||
* | ||
* 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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"? |
||
* @param {Object} props - React props that are set on the HTML element | ||
* @returns {Object} - React props that are valid for the HTML element | ||
* | ||
* @example | ||
* | ||
* const rest = { form: "planDetailsForm", type: "text", placeholder: "Enter plan name", showField: true } | ||
* <span id="planNameError" className="error-message" {...filterInvalidHtmlProps("span", rest)}> | ||
* This field is required! | ||
* </span> | ||
* | ||
* // <span id="planNameError" class="error-message"> | ||
* // This field is required! | ||
* // </span> | ||
* | ||
*/ | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does "covers" mean in this context? |
||
// but it does not filter out HTML attributes that are not allowed for the element in question | ||
const validDOMProps = filterInvalidDOMProps(props) | ||
|
||
const notAllowedAttributes = generateNotAllowedAttributes(element) | ||
return omitBy(validDOMProps, (_, key) => | ||
notAllowedAttributes.includes(htmlAttributes[key]) | ||
) | ||
Comment on lines
+39
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be optimized, but I think we should only look at that after benchmarking It probably does make sense to write specs first, since then you can change the underlying code under green with confidence. |
||
} | ||
|
||
// ----- PRIVATE ------ | ||
// Returns an array of all existing HTML attributes (in plain JavaScript) that are not allowed for the given element | ||
function generateNotAllowedAttributes(element) { | ||
const additionalAllowedAttributes = htmlElementAttributes[element] // e.g., ul => ['compact', 'type'] | ||
const allAllowedAttributes = compact( | ||
concat(basicGlobalAttributes, 'role', additionalAllowedAttributes) | ||
) | ||
return difference(allHtmlAttributes, allAllowedAttributes) | ||
} | ||
|
||
export default filterInvalidHtmlProps |
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.
Is the value the "JavaScript prop" or the html attribute?