Skip to content

Commit

Permalink
Add warning about css shorthand properties collision(facebook#6348)
Browse files Browse the repository at this point in the history
  • Loading branch information
logic0319 committed Oct 16, 2018
1 parent 0af8199 commit 4fe3708
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 0 deletions.
29 changes: 29 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2698,5 +2698,34 @@ describe('ReactDOMComponent', () => {
ReactDOM.render(<Component />, container);
expect(typeof container.onclick).not.toBe('function');
});

it('should warn about css shorthand properties collision', () => {
const container = document.createElement('div');
const oldStyle = {
background: 'url(http://example.org/image/a.jpg) no-repeat center',
backgroundSize: '150px',
backgroundColor: 'red',
};

ReactDOM.render(<div style={oldStyle} />, container);
const stubStyle = container.firstChild.style;

expect(stubStyle.background).toEqual(
'url(http://example.org/image/a.jpg) no-repeat center',
);
expect(stubStyle.backgroundSize).toEqual('150px');
expect(stubStyle.backgroundColor).toEqual('red');

const newStyle = {
background: 'url(http://example.org/image/b.jpg) no-repeat center',
};

expect(() =>
ReactDOM.render(<div style={newStyle} />, container),
).toWarnDev(
'Warning: Css shorthand properties collision:' +
' background properties are being overridden. (backgroundSize,backgroundColor)',
);
});
});
});
21 changes: 21 additions & 0 deletions packages/react-dom/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {registrationNameModules} from 'events/EventPluginRegistry';
import warning from 'shared/warning';
import {canUseDOM} from 'shared/ExecutionEnvironment';
import warningWithoutStack from 'shared/warningWithoutStack';
import {shorthandProperties} from '../shared/CSSProperty';

import * as DOMPropertyOperations from './DOMPropertyOperations';
import * as ReactDOMInput from './ReactDOMInput';
Expand Down Expand Up @@ -709,6 +710,26 @@ export function diffProperties(
if (!styleUpdates) {
styleUpdates = {};
}
// Check shorthand properties collision
if (__DEV__) {
if (shorthandProperties.hasOwnProperty(styleName)) {
const propertyGroup = shorthandProperties[styleName];
const overriddenProperties = [];
for (let lastStyleName in lastProp) {
if (propertyGroup.includes(lastStyleName)) {
overriddenProperties.push(lastStyleName);
}
}
if (overriddenProperties.length > 0) {
warning(
false,
'Css shorthand properties collision: %s properties are being overridden. (%s)',
styleName,
overriddenProperties,
);
}
}
}
styleUpdates[styleName] = nextProp[styleName];
}
}
Expand Down
12 changes: 12 additions & 0 deletions packages/react-dom/src/shared/CSSProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ export const isUnitlessNumber = {
strokeWidth: true,
};

export const shorthandProperties = {
background: [
'backgroundAttachment',
'backgroundClip',
'backgroundColor',
'backgroundImage',
'backgroundOrigin',
'backgroundPosition',
'backgroundRepeat',
'backgroundSize',
],
};
/**
* @param {string} prefix vendor-specific prefix, eg: Webkit
* @param {string} key style name, eg: transitionDuration
Expand Down

0 comments on commit 4fe3708

Please sign in to comment.