Skip to content
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

fix non-boolean attributes #138

Merged
merged 4 commits into from
Aug 6, 2021
Merged
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
39 changes: 28 additions & 11 deletions src/index.browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function toReactNode(
attrs.key = key.toString();

const tag = node.tagName.toLowerCase() as HTMLTags;
const props = mapAttribute(tag, attrs, preserveAttributes, getPropName);
const props = mapAttribute(tag, attrs, preserveAttributes, getPropInfo);

const children = Array.from(node.childNodes)
.map((childNode, i) => {
Expand Down Expand Up @@ -108,12 +108,15 @@ function toReactNode(
return reactCreateElement(tag, props, transform, reactChildren);
}

const specialPropsMap: Record<string, string> = {
// map HTML attribute to react props, and optionally DOM prop by using array
// if DOM prop is same as attribute name, use single item array
const attributePropMap: Record<string, string | string[]> = {
for: 'htmlFor',
class: 'className',
allowfullscreen: 'allowFullScreen',
// react prop and DOM prop have different casing
allowfullscreen: ['allowFullScreen', 'allowFullscreen'],
autocomplete: 'autoComplete',
autofocus: 'autoFocus',
autofocus: ['autoFocus'],
contenteditable: 'contentEditable',
spellcheck: 'spellCheck',
srcdoc: 'srcDoc',
Expand All @@ -127,21 +130,35 @@ const specialPropsMap: Record<string, string> = {
*
* For other edge cases we can use specialPropsMaps
*/
function getPropName(originalTag: HTMLTags, attributeName: string): string {
function getPropInfo(tagName: HTMLTags, attributeName: string) {
const propName = attributePropMap[attributeName];
const el = document.createElement(tagName);

// handle edge cases first
if (specialPropsMap[attributeName]) {
return specialPropsMap[attributeName];
if (propName) {
const reactProp = Array.isArray(propName) ? propName[0] : propName;
const domProp = Array.isArray(propName)
? propName[1] || attributeName
: propName;
return { name: reactProp, isBoolean: checkBooleanAttribute(el, domProp) };
}

const el = document.createElement(originalTag);

for (let propName in el) {
if (propName.toLowerCase() === attributeName.toLowerCase()) {
return propName;
return { name: propName, isBoolean: checkBooleanAttribute(el, propName) };
}
}

return attributeName;
return {
name: attributeName,
isBoolean: checkBooleanAttribute(el, attributeName),
};
}

function checkBooleanAttribute(el: HTMLElement, prop: any) {
el.setAttribute(prop, '');
// @ts-ignore
return el[prop] === true;
}

function reactCreateElement(
Expand Down
40 changes: 37 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function toReactNode(

const props = Object.assign(
{},
mapAttribute(name, attribs, preserveAttributes, getPropName),
mapAttribute(name, attribs, preserveAttributes, getPropInfo),
{ key }
);

Expand Down Expand Up @@ -125,6 +125,40 @@ import attributes from './attribute.json';
type AttributeMap = Record<string, string>;
const attrs = <AttributeMap>attributes;

function getPropName(_originalTag: HTMLTags, attributeName: string) {
return attrs[attributeName] || attributeName;
function getPropInfo(_originalTag: HTMLTags, attributeName: string) {
const propName = attrs[attributeName] || attributeName;
return {
name: propName,
isBoolean: BOOLEAN_ATTRIBUTES.includes(propName),
};
}

const BOOLEAN_ATTRIBUTES = [
// https://github.com/facebook/react/blob/cae635054e17a6f107a39d328649137b83f25972/packages/react-dom/src/shared/DOMProperty.js#L319
'allowFullScreen',
'async',
// Note: there is a special case that prevents it from being written to the DOM
// on the client side because the browsers are inconsistent. Instead we call focus().
'autoFocus',
'autoPlay',
'controls',
'default',
'defer',
'disabled',
'disablePictureInPicture',
'disableRemotePlayback',
'formNoValidate',
'hidden',
'loop',
'noModule',
'noValidate',
'open',
'playsInline',
'readOnly',
'required',
'reversed',
'scoped',
'seamless',
// Microdata
'itemScope',
];
15 changes: 9 additions & 6 deletions src/mapAttribute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ export default function mapAttribute(
originalTag: HTMLTags,
attrs = {} as RawAttributes,
preserveAttributes: Array<String | RegExp>,
getPropName: (originalTag: HTMLTags, attributeName: string) => string
getPropInfo: (
originalTag: HTMLTags,
attributeName: string
) => { name: string; isBoolean: boolean }
): Attributes {
return Object.keys(attrs).reduce((result, attr) => {
// ignore inline event attribute
Expand All @@ -39,19 +42,19 @@ export default function mapAttribute(
}
}

const name = getPropName(originalTag, attributeName);
if (name === 'style') {
const prop = getPropInfo(originalTag, attributeName);
if (prop.name === 'style') {
// if there's an attribute called style, this means that the value must be exists
// even if it's an empty string
result[name] = convertStyle(attrs.style!);
result[prop.name] = convertStyle(attrs.style!);
} else {
const value = attrs[attr];
// Convert attribute value to boolean attribute if needed
// https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes
const isBooleanAttribute =
const booleanAttrributeValue =
value === '' ||
String(value).toLowerCase() === attributeName.toLowerCase();
result[name] = isBooleanAttribute ? true : value;
result[prop.name] = prop.isBoolean ? booleanAttrributeValue : value;
}

return result;
Expand Down
9 changes: 8 additions & 1 deletion test/__snapshots__/htmr.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ exports[`attributes correctly map HTML attributes to react props 1`] = `
exports[`attributes correctly map HTML attributes to react props 2`] = `
<div
aria-describedby="info"
contentEditable={true}
contentEditable="true"
data-type="calendar"
id="test"
spellCheck="true"
Expand Down Expand Up @@ -119,6 +119,13 @@ exports[`attributes correctly map HTML attributes to react props 9`] = `
</time>
`;

exports[`attributes correctly map HTML attributes to react props 10`] = `
<img
alt="alt"
className=""
/>
`;

exports[`attributes decode HTML entities inside style declaration 1`] = `
<em
style={
Expand Down
14 changes: 10 additions & 4 deletions test/htmr.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('attributes', () => {
test('correctly map HTML attributes to react props', () => {
testRender('<label class="input-text" for="name"></label>');
testRender(
'<div id="test" data-type="calendar" aria-describedby="info" spellcheck="true" contenteditable></div>'
'<div id="test" data-type="calendar" aria-describedby="info" spellcheck="true" contenteditable="true"></div>'
);
testRender('<link xml:lang="en" xlink:actuate="other" />');
testRender(oneLineTrim`
Expand All @@ -74,14 +74,20 @@ describe('attributes', () => {
);
testRender('<button accesskey="s">Stress reliever</button>');
testRender('<time datetime="2018-07-07">July 7</time>');
testRender('<img alt="alt" class="" />');
});

// https://github.com/pveyes/htmr/issues/103
test('correctly handle boolean attributes', () => {
const { container } = render(htmrBrowser('<iframe allowfullscreen />'));
expect(
container.querySelector('iframe').getAttribute('allowfullscreen')
).toEqual('');
expect(container.querySelector('iframe').allowFullscreen).toBe(true);
});

// https://github.com/pveyes/htmr/issues/137
test('pass non-boolean attributes as is', () => {
const { container } = render(htmrBrowser('<img alt="alt" class="" />'));
expect(container.querySelector('img').alt).toEqual('alt');
expect(container.querySelector('img').className).toEqual('');
});

test('correctly convert multiple style values', () => {
Expand Down