Skip to content

Commit

Permalink
Add DOM warnings by calling hooks directly
Browse files Browse the repository at this point in the history
It is not clear if the old hook system is worth it in its generic incarnation. For now I am just hooking it up to the DOMFiber renderer directly.
  • Loading branch information
gaearon committed Dec 14, 2016
1 parent 0aeda52 commit 05bf2e3
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 64 deletions.
19 changes: 0 additions & 19 deletions scripts/fiber/tests-passing-except-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,14 @@ src/addons/transitions/__tests__/ReactTransitionGroup-test.js
* should warn for duplicated keys with component stack info

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
* should warn for unknown prop
* should group multiple unknown prop warnings together
* should warn for onDblClick prop
* should not warn when server-side rendering `onScroll`
* warns on invalid nesting
* warns on invalid nesting at root
* warns nicely for table rows
* gives useful context in warnings
* should warn about incorrect casing on properties (ssr)
* should warn about incorrect casing on event handlers (ssr)
* should warn about incorrect casing on properties
* should warn about incorrect casing on event handlers
* should warn about class
* should suggest property name if available

src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js
* should warn for one invalid aria-* prop
* should warn for many invalid aria-* props
* should warn for an improperly cased aria-* prop

src/renderers/dom/shared/__tests__/ReactMount-test.js
* should warn if mounting into dirty rendered markup
Expand All @@ -37,15 +26,7 @@ src/renderers/dom/shared/__tests__/ReactServerRendering-test.js
* should have the correct mounting behavior

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should warn if value is null
* should warn if controlled input switches to uncontrolled (value is null)
* should warn if uncontrolled input (value is null) switches to controlled

src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js
* should warn if value is null

src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js
* should warn if value is null

src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js
* uses displayName or Unknown for classic components
Expand Down
13 changes: 13 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,9 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
* should gracefully handle various style value types
* should not update styles when mutating a proxy style object
* should throw when mutating style objectsd
* should warn for unknown prop
* should group multiple unknown prop warnings together
* should warn for onDblClick prop
* should not warn for "0" as a unitless style value
* should warn nicely about NaN in style
* should update styles if initially null
Expand Down Expand Up @@ -687,7 +690,10 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
* should throw when an attack vector is used server-side
* should throw when an invalid tag name is used
* should throw when an attack vector is used
* should warn about incorrect casing on properties
* should warn about incorrect casing on event handlers
* should warn about props that are no longer supported
* should suggest property name if available

src/renderers/dom/shared/__tests__/ReactDOMComponentTree-test.js
* finds nodes for instances
Expand All @@ -698,6 +704,9 @@ src/renderers/dom/shared/__tests__/ReactDOMIDOperations-test.js

src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js
* should allow valid aria-* props
* should warn for one invalid aria-* prop
* should warn for many invalid aria-* props
* should warn for an improperly cased aria-* prop

src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js
* creates initial namespaced markup
Expand Down Expand Up @@ -951,11 +960,13 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should have a this value of undefined if bind is not used
* should warn with checked and no onChange handler with readOnly specified
* should update defaultValue to empty string
* should warn if value is null
* should warn if checked and defaultChecked props are specified
* should warn if value and defaultValue props are specified
* should warn if controlled input switches to uncontrolled (value is undefined)
* should warn if controlled input switches to uncontrolled with defaultValue
* should warn if uncontrolled input (value is undefined) switches to controlled
* should warn if uncontrolled input (value is null) switches to controlled
* should warn if controlled checkbox switches to uncontrolled (checked is undefined)
* should warn if controlled checkbox switches to uncontrolled (checked is null)
* should warn if controlled checkbox switches to uncontrolled with defaultChecked
Expand Down Expand Up @@ -1001,6 +1012,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js
* should support server-side rendering with defaultValue
* should support server-side rendering with multiple
* should not control defaultValue if readding options
* should warn if value is null
* should refresh state on change
* should warn if value and defaultValue props are specified
* should be able to safely remove select onChange
Expand Down Expand Up @@ -1033,6 +1045,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js
* should allow objects as children
* should throw with multiple or invalid children
* should unmount
* should warn if value is null
* should warn if value and defaultValue are specified

src/renderers/native/__tests__/ReactNativeAttributePayload-test.js
Expand Down
9 changes: 9 additions & 0 deletions src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ var {
} = ReactDOMFiberComponent;
var { precacheFiberNode } = ReactDOMComponentTree;

if (__DEV__) {
var ReactDOMInvalidARIAHook = require('ReactDOMInvalidARIAHook');
var ReactDOMNullInputValuePropHook = require('ReactDOMNullInputValuePropHook');
var ReactDOMUnknownPropertyHook = require('ReactDOMUnknownPropertyHook');
}

const DOCUMENT_NODE = 9;

ReactDOMInjection.inject();
Expand Down Expand Up @@ -104,6 +110,9 @@ var DOMRenderer = ReactFiberReconciler({
) : void {
if (__DEV__) {
validateProperties(domElement, type, null, props);
ReactDOMInvalidARIAHook.validateProperties(type, props);
ReactDOMNullInputValuePropHook.validateProperties(type, props);
ReactDOMUnknownPropertyHook.validateProperties(type, props);
}
setInitialProperties(domElement, type, props, rootContainerInstance);
},
Expand Down
48 changes: 29 additions & 19 deletions src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,23 @@

var DOMProperty = require('DOMProperty');
var ReactComponentTreeHook = require('ReactComponentTreeHook');
var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');

var warning = require('warning');

var warnedProperties = {};
var rARIA = new RegExp('^(aria)-[' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$');

function getStackAddendum(debugID) {
if (debugID != null) {
// This can only happen on Stack
return ReactComponentTreeHook.getStackAddendumByID(debugID);
} else {
// This can only happen on Fiber
return ReactDebugCurrentFiber.getCurrentFiberStackAddendum();
}
}

function validateProperty(tagName, name, debugID) {
if (
warnedProperties.hasOwnProperty(name)
Expand Down Expand Up @@ -47,7 +58,7 @@ function validateProperty(tagName, name, debugID) {
'Unknown ARIA attribute %s. Did you mean %s?%s',
name,
standardName,
ReactComponentTreeHook.getStackAddendumByID(debugID)
getStackAddendum(debugID)
);
warnedProperties[name] = true;
return true;
Expand All @@ -57,11 +68,11 @@ function validateProperty(tagName, name, debugID) {
return true;
}

function warnInvalidARIAProps(debugID, element) {
function warnInvalidARIAProps(type, props, debugID) {
const invalidProps = [];

for (var key in element.props) {
var isValid = validateProperty(element.type, key, debugID);
for (var key in props) {
var isValid = validateProperty(type, key, debugID);
if (!isValid) {
invalidProps.push(key);
}
Expand All @@ -77,41 +88,40 @@ function warnInvalidARIAProps(debugID, element) {
'Invalid aria prop %s on <%s> tag. ' +
'For details, see https://fb.me/invalid-aria-prop%s',
unknownPropString,
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
type,
getStackAddendum(debugID)
);
} else if (invalidProps.length > 1) {
warning(
false,
'Invalid aria props %s on <%s> tag. ' +
'For details, see https://fb.me/invalid-aria-prop%s',
unknownPropString,
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
type,
getStackAddendum(debugID)
);
}
}

function handleElement(debugID, element) {
if (element == null || typeof element.type !== 'string') {
function validateProperties(type, props, debugID /* Stack only */) {
if (type.indexOf('-') >= 0 || props.is) {
return;
}
if (element.type.indexOf('-') >= 0 || element.props.is) {
return;
}

warnInvalidARIAProps(debugID, element);
warnInvalidARIAProps(type, props, debugID);
}

var ReactDOMInvalidARIAHook = {
// Fiber
validateProperties,
// Stack
onBeforeMountComponent(debugID, element) {
if (__DEV__) {
handleElement(debugID, element);
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
}
},
onBeforeUpdateComponent(debugID, element) {
if (__DEV__) {
handleElement(debugID, element);
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
}
},
};
Expand Down
33 changes: 24 additions & 9 deletions src/renderers/dom/shared/hooks/ReactDOMNullInputValuePropHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,38 +12,53 @@
'use strict';

var ReactComponentTreeHook = require('ReactComponentTreeHook');
var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');

var warning = require('warning');

var didWarnValueNull = false;

function handleElement(debugID, element) {
if (element == null) {
return;
function getStackAddendum(debugID) {
if (debugID != null) {
// This can only happen on Stack
return ReactComponentTreeHook.getStackAddendumByID(debugID);
} else {
// This can only happen on Fiber
return ReactDebugCurrentFiber.getCurrentFiberStackAddendum();
}
if (element.type !== 'input' && element.type !== 'textarea' && element.type !== 'select') {
}

function validateProperties(type, props, debugID /* Stack only */) {
if (type !== 'input' && type !== 'textarea' && type !== 'select') {
return;
}
if (element.props != null && element.props.value === null && !didWarnValueNull) {
if (props != null && props.value === null && !didWarnValueNull) {
warning(
false,
'`value` prop on `%s` should not be null. ' +
'Consider using the empty string to clear the component or `undefined` ' +
'for uncontrolled components.%s',
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
type,
getStackAddendum(debugID)
);

didWarnValueNull = true;
}
}

var ReactDOMNullInputValuePropHook = {
// Fiber
validateProperties,
// Stack
onBeforeMountComponent(debugID, element) {
handleElement(debugID, element);
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
}
},
onBeforeUpdateComponent(debugID, element) {
handleElement(debugID, element);
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
}
},
};

Expand Down
49 changes: 32 additions & 17 deletions src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,20 @@
var DOMProperty = require('DOMProperty');
var EventPluginRegistry = require('EventPluginRegistry');
var ReactComponentTreeHook = require('ReactComponentTreeHook');
var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');

var warning = require('warning');

function getStackAddendum(debugID) {
if (debugID != null) {
// This can only happen on Stack
return ReactComponentTreeHook.getStackAddendumByID(debugID);
} else {
// This can only happen on Fiber
return ReactDebugCurrentFiber.getCurrentFiberStackAddendum();
}
}

if (__DEV__) {
var reactProps = {
children: true,
Expand Down Expand Up @@ -71,7 +82,7 @@ if (__DEV__) {
'Unknown DOM property %s. Did you mean %s?%s',
name,
standardName,
ReactComponentTreeHook.getStackAddendumByID(debugID)
getStackAddendum(debugID)
);
return true;
} else if (registrationName != null) {
Expand All @@ -80,7 +91,7 @@ if (__DEV__) {
'Unknown event handler property %s. Did you mean `%s`?%s',
name,
registrationName,
ReactComponentTreeHook.getStackAddendumByID(debugID)
getStackAddendum(debugID)
);
return true;
} else {
Expand All @@ -93,10 +104,10 @@ if (__DEV__) {
};
}

var warnUnknownProperties = function(debugID, element) {
var warnUnknownProperties = function(type, props, debugID) {
var unknownProps = [];
for (var key in element.props) {
var isValid = validateProperty(element.type, key, debugID);
for (var key in props) {
var isValid = validateProperty(type, key, debugID);
if (!isValid) {
unknownProps.push(key);
}
Expand All @@ -112,37 +123,41 @@ var warnUnknownProperties = function(debugID, element) {
'Unknown prop %s on <%s> tag. Remove this prop from the element. ' +
'For details, see https://fb.me/react-unknown-prop%s',
unknownPropString,
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
type,
getStackAddendum(debugID)
);
} else if (unknownProps.length > 1) {
warning(
false,
'Unknown props %s on <%s> tag. Remove these props from the element. ' +
'For details, see https://fb.me/react-unknown-prop%s',
unknownPropString,
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
type,
getStackAddendum(debugID)
);
}
};

function handleElement(debugID, element) {
if (element == null || typeof element.type !== 'string') {
function validateProperties(type, props, debugID /* Stack only */) {
if (type.indexOf('-') >= 0 || props.is) {
return;
}
if (element.type.indexOf('-') >= 0 || element.props.is) {
return;
}
warnUnknownProperties(debugID, element);
warnUnknownProperties(type, props, debugID);
}

var ReactDOMUnknownPropertyHook = {
// Fiber
validateProperties,
// Stack
onBeforeMountComponent(debugID, element) {
handleElement(debugID, element);
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
}
},
onBeforeUpdateComponent(debugID, element) {
handleElement(debugID, element);
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
}
},
};

Expand Down

0 comments on commit 05bf2e3

Please sign in to comment.