Skip to content

Commit

Permalink
Validate styles during reconciliation
Browse files Browse the repository at this point in the history
Previously, Fiber validated styles at the same time setting properties. However, since properties get updated during the commit phase, validation in updates did not have access to the current owner.

We have moved validation to prepareUpdate() since it has all the necessary information. Now Fiber has access to the owner when validating styles. We can add more validations to the same place later.
  • Loading branch information
gaearon committed Dec 14, 2016
1 parent ff9cdf1 commit 1577957
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 6 deletions.
3 changes: 0 additions & 3 deletions scripts/fiber/tests-passing-except-dev.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
src/addons/transitions/__tests__/ReactTransitionGroup-test.js
* should warn for duplicated keys with component stack info

src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
* should warn when updating hyphenated style names

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
* should warn for unknown prop
* should group multiple unknown prop warnings together
Expand Down
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
* should set style attribute when styles exist
* should not set style attribute when no styles exist
* should warn when using hyphenated style names
* should warn when updating hyphenated style names
* warns when miscapitalizing vendored style names
* should warn about style having a trailing semicolon
* should warn about style containing a NaN value
Expand Down
7 changes: 7 additions & 0 deletions src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var {
createElement,
getChildNamespace,
setInitialProperties,
validateProperties,
updateProperties,
} = ReactDOMFiberComponent;
var { precacheFiberNode } = ReactDOMComponentTree;
Expand Down Expand Up @@ -101,6 +102,9 @@ var DOMRenderer = ReactFiberReconciler({
props : Props,
rootContainerInstance : Container,
) : void {
if (__DEV__) {
validateProperties(domElement, type, null, props);
}
setInitialProperties(domElement, type, props, rootContainerInstance);
},

Expand All @@ -110,6 +114,9 @@ var DOMRenderer = ReactFiberReconciler({
oldProps : Props,
newProps : Props
) : boolean {
if (__DEV__) {
validateProperties(domElement, type, oldProps, newProps);
}
return true;
},

Expand Down
43 changes: 40 additions & 3 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -423,16 +423,38 @@ function updateDOMProperties(
}
}
if (styleUpdates) {
if (__DEV__) {
CSSPropertyOperations.warnValidStyles(styleUpdates);
}
CSSPropertyOperations.setValueForStyles(
domElement,
styleUpdates,
);
}
}

function validateStyles(lastStyle : Object | null, nextStyle : Object) {
if (__DEV__) {
let styleUpdates = null;
// Find all styles that changed.
if (lastStyle != null) {
for (let styleName in nextStyle) {
if (
nextStyle.hasOwnProperty(styleName) &&
lastStyle[styleName] !== nextStyle[styleName]
) {
styleUpdates = styleUpdates || {};
styleUpdates[styleName] = nextStyle[styleUpdates];
}
}
if (styleUpdates == null) {
// Nothing changed.
return;
}
} else {
styleUpdates = nextStyle;
}
CSSPropertyOperations.warnValidStyles(styleUpdates);
}
}

// Assumes there is no parent namespace.
function getIntrinsicNamespace(type : string) : string | null {
switch (type) {
Expand Down Expand Up @@ -693,6 +715,21 @@ var ReactDOMFiberComponent = {
}
},

validateProperties(
domElement : Element,
tag : string,
lastProps : Object | null,
nextProps : Object,
) : void {
if (__DEV__) {
const nextStyle = nextProps.style;
if (nextStyle != null) {
const lastStyle = lastProps == null ? null : lastProps.style;
validateStyles(lastStyle, nextStyle);
}
}
},

restoreControlledState(domElement : Element, tag : string, props : Object) : void {
switch (tag) {
case 'input':
Expand Down

0 comments on commit 1577957

Please sign in to comment.