adobe / react-webcomponent

This projects automates the wrapping of a React component in a CustomElement.
Apache License 2.0
105 stars 19 forks source link

Please consider changing behavior of `byBooleanAttrVal` #18

Closed charlessuh closed 4 years ago

charlessuh commented 5 years ago

Description

The behavior of the current byBooleanAttrVal returns true or false based on the existence of an attribute. This change renames that behavior to byExistAttrVal.

The new byBooleanAttrVal will return true or false based on the literal string value (e.g. "true" or "false") and will return undefined for other cases, including lack of existence of the attribute.

Related Issue

https://github.com/adobe/react-webcomponent/issues/13

Motivation and Context

If you create a React component with an optional boolean prop that defaults to true, the current behavior of byBooleanAttrVal looks at an attribute being missing and passes an explicit false to the React component, short-circuiting the default handling logic.

It would be desirable in this case to pass undefined to the React component, so the default handling logic works.

How Has This Been Tested?

npm run test

Screenshots (if appropriate):

n/a

Types of changes

Checklist:

alexmirea commented 5 years ago

Hi Charles!

Thanks for the PR. The byBooleanAttrVal works according to the spec https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes I don't want to break the API, so what we can do is add a parameter to the byBooleanAttrVal to send undefined when not present, and to document this.