Skip to content

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
474 changes: 254 additions & 220 deletions docs.md

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
"cleave.js": "^1.6.0",
"date-fns": "^2.28.0",
"filter-invalid-dom-props": "^2.1.0",
"html-attributes": "^1.1.0",
"html-element-attributes": "^3.1.0",
"lodash": "^4.17.21",
"prop-types": "^15.8.1",
"react-color": "^2.19.3",
Expand Down
4 changes: 2 additions & 2 deletions src/forms/labels/input-error.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react'
import PropTypes from 'prop-types'
import classnames from 'classnames'
import { generateInputErrorId, filterInvalidDOMProps } from '../../utils'
import { generateInputErrorId, filterInvalidHtmlProps } from '../../utils'
import { hasInputError } from '../helpers'

/**
Expand Down Expand Up @@ -75,7 +75,7 @@ function InputError({ error, invalid, touched, name, className, ...rest }) {
<span
id={generateInputErrorId(name)}
className={classnames('error-message', className)}
{...filterInvalidDOMProps(rest)}
{...filterInvalidHtmlProps('span', rest)}
>
{formatError(error)}
</span>
Expand Down
54 changes: 54 additions & 0 deletions src/utils/local/filter-invalid-html-props.js
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
Copy link
Contributor

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?

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-")
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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-"
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 filterInvalidHtmlProps against filterinvalidDOMProps.

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
1 change: 1 addition & 0 deletions src/utils/local/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ export stripNamespace from './strip-namespace'
export triggerOnKeys from './trigger-on-keys'
export KeyCodes from './key-codes'
export useToggle from './use-toggle'
export filterInvalidHtmlProps from './filter-invalid-html-props'
7 changes: 6 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8109,11 +8109,16 @@ hosted-git-info@^4.0.0, hosted-git-info@^4.0.1:
dependencies:
lru-cache "^6.0.0"

html-attributes@1.1.0:
html-attributes@1.1.0, html-attributes@^1.1.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/html-attributes/-/html-attributes-1.1.0.tgz#82027a4fac7a6070ea6c18cc3886aea18d6dea09"
integrity sha1-ggJ6T6x6YHDqbBjMOIauoY1t6gk=

html-element-attributes@^3.1.0:
version "3.1.0"
resolved "https://registry.yarnpkg.com/html-element-attributes/-/html-element-attributes-3.1.0.tgz#07869037b9020bec3bb1897263e97dd65829d15b"
integrity sha512-cHM9qM06tyWHwvGqDqVEBwoYtGgyq7X/GQt3dor38M1hYMZw1yVadaDQrwwQer6NefiYAoHaqFARI8ETMCAOYA==

html-element-map@^1.2.0:
version "1.3.1"
resolved "https://registry.yarnpkg.com/html-element-map/-/html-element-map-1.3.1.tgz#44b2cbcfa7be7aa4ff59779e47e51012e1c73c08"
Expand Down