Skip to content

Commit

Permalink
Allow uppercase letters in custom attributes. Address associated edge…
Browse files Browse the repository at this point in the history
… cases
  • Loading branch information
nhunzaker committed Aug 7, 2017
1 parent facfa87 commit eb2e759
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 49 deletions.
3 changes: 1 addition & 2 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Expand Up @@ -922,8 +922,7 @@ var ReactDOMFiberComponent = {
var extraAttributeNames: Set<string> = new Set();
var attributes = domElement.attributes;
for (var i = 0; i < attributes.length; i++) {
// TODO: Do we need to lower case this to get case insensitive matches?
var name = attributes[i].name;
var name = attributes[i].name.toLowerCase();
switch (name) {
// Built-in attributes are whitelisted
// TODO: Once these are gone from the server renderer, we don't need
Expand Down
10 changes: 5 additions & 5 deletions src/renderers/dom/shared/DOMProperty.js
Expand Up @@ -119,7 +119,11 @@ var DOMPropertyInjection = {
if (DOMAttributeNames.hasOwnProperty(propName)) {
var attributeName = DOMAttributeNames[propName];

DOMProperty.aliases[attributeName] = true;
DOMProperty.aliases[attributeName.toLowerCase()] = true;

if (lowerCased !== attributeName) {
DOMProperty.aliases[lowerCased] = true;
}

propertyInfo.attributeName = attributeName;
if (__DEV__) {
Expand Down Expand Up @@ -238,10 +242,6 @@ var DOMProperty = {
return true;
}

if (name.toLowerCase() !== name) {
return false;
}

switch (typeof value) {
case 'undefined':
case 'boolean':
Expand Down
24 changes: 4 additions & 20 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Expand Up @@ -1992,30 +1992,14 @@ describe('ReactDOMComponent', () => {
expect(el.getAttribute('ajaxify')).toBe('ajaxy');
});

it('only allows lower-case data attributes', function() {
spyOn(console, 'error');

it('allows cased data attributes', function() {
var el = ReactTestUtils.renderIntoDocument(<div data-fooBar="true" />);

expect(el.hasAttribute('data-foobar')).toBe(false);

expectDev(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Invalid DOM property `data-fooBar`. Custom attributes ' +
'and data attributes must be lower case. Instead use `data-foobar`',
);
expect(el.getAttribute('data-foobar')).toBe('true');
});

it('only allows lower-case custom attributes', function() {
spyOn(console, 'error');

it('allows cased custom attributes', function() {
var el = ReactTestUtils.renderIntoDocument(<div fooBar="true" />);

expect(el.hasAttribute('foobar')).toBe(false);

expectDev(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Invalid DOM property `fooBar`. Custom attributes ' +
'and data attributes must be lower case. Instead use `foobar`',
);
expect(el.getAttribute('foobar')).toBe('true');
});
});
});
Expand Up @@ -615,6 +615,12 @@ describe('ReactDOMServerIntegration', () => {
expect(e.hasAttribute('className')).toBe(false);
});

itRenders('no classname prop', async render => {
const e = await render(<div classname="test" />, 1);
expect(e.hasAttribute('class')).toBe(false);
expect(e.hasAttribute('classname')).toBe(false);
});

itRenders('no className prop when given the alias', async render => {
const e = await render(<div class="test" />, 1);
expect(e.className).toBe('');
Expand Down Expand Up @@ -793,9 +799,32 @@ describe('ReactDOMServerIntegration', () => {
});
});

describe('cased attributes', function() {
itRenders('no badly cased aliased HTML attribute', async render => {
const e = await render(<meta httpequiv="refresh" />, 1);
expect(e.hasAttribute('http-equiv')).toBe(false);
expect(e.hasAttribute('httpequiv')).toBe(false);
});

itRenders('no badly cased SVG attribute', async render => {
const e = await render(<text textlength="10" />, 1);
expect(e.hasAttribute('textLength')).toBe(false);
});

itRenders('no badly cased aliased SVG attribute', async render => {
const e = await render(<text strokedasharray="10 10" />, 1);
expect(e.hasAttribute('strokedasharray')).toBe(false);
});

itRenders('no badly cased original SVG attribute that is aliased', async render => {
const e = await render(<text stroke-dasharray="10 10" />, 1);
expect(e.hasAttribute('stroke-dasharray')).toBe(false);
});
});

describe('unknown attributes', function() {
itRenders('unknown attributes', async render => {
const e = await render(<div foo="bar" />, 0);
const e = await render(<div foo="bar" />);
expect(e.getAttribute('foo')).toBe('bar');
});

Expand All @@ -809,13 +838,21 @@ describe('ReactDOMServerIntegration', () => {
expect(e.hasAttribute('data-foo')).toBe(false);
});

itRenders('no unknown data- attributes with casing', async render => {
const e = await render(<div data-fooBar={null} />, 1);
expect(e.hasAttribute('data-foobar')).toBe(false);
itRenders('unknown data- attributes with casing', async render => {
const e = await render(<div data-fooBar="true" />);
expect(e.getAttribute('data-fooBar')).toBe('true');
});

itRenders(
'no unknown data- attributes with casing and null value',
async render => {
const e = await render(<div data-fooBar={null} />);
expect(e.hasAttribute('data-fooBar')).toBe(false);
},
);

itRenders('custom attributes for non-standard elements', async render => {
const e = await render(<nonstandard foo="bar" />, 0);
const e = await render(<nonstandard foo="bar" />);
expect(e.getAttribute('foo')).toBe('bar');
});

Expand Down Expand Up @@ -848,9 +885,9 @@ describe('ReactDOMServerIntegration', () => {
},
);

itRenders('no cased custom attributes', async render => {
const e = await render(<div fooBar="test" />, 1);
expect(e.hasAttribute('fooBar')).toBe(false);
itRenders('cased custom attributes', async render => {
const e = await render(<div fooBar="test" />);
expect(e.getAttribute('fooBar')).toBe('test');
});
});

Expand Down
14 changes: 0 additions & 14 deletions src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js
Expand Up @@ -101,20 +101,6 @@ if (__DEV__) {
return true;
}

// Otherwise, we have a custom attribute. Custom attributes should always
// be lowercase.
if (lowerCasedName !== name) {
warning(
false,
'Invalid DOM property `%s`. Custom attributes and data attributes ' +
'must be lower case. Instead use `%s`.%s',
name,
lowerCasedName,
getStackAddendum(debugID),
);
return true;
}

return DOMProperty.shouldSetAttribute(name, value);
};
}
Expand Down

0 comments on commit eb2e759

Please sign in to comment.