Add warning for shady-dom use with rendered react components in DEV (#7911)
* Add warning for shady-dom use with rendered react components in DEV * Make shady dom warning emit only once regardless of number of components * Use didWarnShadyDom as warning argument & add missing shadyRoot check to ReactDOMComponent * Add check for shady dom test to run only when createElement flag is true * Pass false to warning in shadyDom warning check * Add consistent DOM phrasing for shady DOM warnings * Reference component name in shady DOM warning if it exists * Add check for if owner exists in shady dom warning * Add test for named component using createClass for shady DOM warning * Clean up named component test for shady DOM warning * Fix trailing comma linting issue on named shady DOM warning test
Kevin Huang committed
Oct 12, 2016 at 02:13 UTC
b728fa0c1796f1050399386d168816654c76c1c1
2 files changed
+85
-4
src/renderers/dom/shared/ReactDOMComponent.js
+23
-4
@@ -40,6 +40,7 @@ var shallowEqual = require('shallowEqual');
40
var inputValueTracking = require('inputValueTracking');
41
var validateDOMNesting = require('validateDOMNesting');
42
var warning = require('warning');
43
+var didWarnShadyDOM = false;
44
45
var Flags = ReactDOMComponentFlags;
46
var deleteListener = EventPluginHub.deleteListener;
@@ -632,12 +633,24 @@ ReactDOMComponent.Mixin = {
633
type
634
);
635
}
636
+ var isCustomComponentTag = isCustomComponent(this._tag, props);
637
+ if (__DEV__ && isCustomComponentTag && !didWarnShadyDOM && el.shadyRoot) {
638
+ var owner = this._currentElement._owner;
639
+ var name = owner && owner.getName() || 'A component';
640
+ warning(
641
+ false,
642
+ '%s is using shady DOM. Using shady DOM with React can ' +
643
+ 'cause things to break subtly.',
644
+ name
645
+ );
646
+ didWarnShadyDOM = true;
647
+ }
648
ReactDOMComponentTree.precacheNode(this, el);
649
this._flags |= Flags.hasCachedChildNodes;
650
if (!this._hostParent) {
651
DOMPropertyOperations.setAttributeForRoot(el);
652
}
640
- this._updateDOMProperties(null, props, transaction);
653
+ this._updateDOMProperties(null, props, transaction, isCustomComponentTag);
654
var lazyTree = DOMLazyTree(el);
655
this._createInitialChildren(transaction, props, context, lazyTree);
656
mountImage = lazyTree;
@@ -901,7 +914,8 @@ ReactDOMComponent.Mixin = {
914
}
915
916
assertValidProps(this, nextProps);
904
- this._updateDOMProperties(lastProps, nextProps, transaction);
917
+ var isCustomComponentTag = isCustomComponent(this._tag, nextProps);
918
+ this._updateDOMProperties(lastProps, nextProps, transaction, isCustomComponentTag);
919
this._updateDOMChildren(
920
lastProps,
921
nextProps,
@@ -943,7 +957,12 @@ ReactDOMComponent.Mixin = {
957
* @param {object} nextProps
958
* @param {?DOMElement} node
959
*/
946
- _updateDOMProperties: function(lastProps, nextProps, transaction) {
960
+ _updateDOMProperties: function(
961
+ lastProps,
962
+ nextProps,
963
+ transaction,
964
+ isCustomComponentTag
965
+ ) {
966
var propKey;
967
var styleName;
968
var styleUpdates;
@@ -1033,7 +1052,7 @@ ReactDOMComponent.Mixin = {
1052
} else if (lastProp) {
1053
deleteListener(this, propKey);
1054
}
1036
- } else if (isCustomComponent(this._tag, nextProps)) {
1055
+ } else if (isCustomComponentTag) {
1056
if (!RESERVED_PROPS.hasOwnProperty(propKey)) {
1057
DOMPropertyOperations.setValueForAttribute(
1058
getNode(this),
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+62
@@ -791,6 +791,68 @@ describe('ReactDOMComponent', () => {
791
);
792
});
793
794
+ it('should emit a warning once for a named custom component using shady DOM', () => {
795
+ if (ReactDOMFeatureFlags.useCreateElement) {
796
+ spyOn(console, 'error');
797
+
798
+ var defaultCreateElement = document.createElement.bind(document);
799
+
800
+ try {
801
+ document.createElement = element => {
802
+ var container = defaultCreateElement(element);
803
+ container.shadyRoot = {};
804
+ return container;
805
+ };
806
+ var ShadyComponent = React.createClass({
807
+ render() {
808
+ return <polymer-component />;
809
+ },
810
+ });
811
+ var node = document.createElement('div');
812
+ ReactDOM.render(<ShadyComponent />, node);
813
+ expect(console.error.calls.count()).toBe(1);
814
+ expect(console.error.calls.argsFor(0)[0]).toContain(
815
+ 'ShadyComponent is using shady DOM. Using shady DOM with React can ' +
816
+ 'cause things to break subtly.'
817
+ );
818
+ mountComponent({is: 'custom-shady-div2'});
819
+ expect(console.error.calls.count()).toBe(1);
820
+
821
+ } finally {
822
+ document.createElement = defaultCreateElement;
823
+ }
824
+ }
825
+ });
826
+
827
+ it('should emit a warning once for an unnamed custom component using shady DOM', () => {
828
+ if (ReactDOMFeatureFlags.useCreateElement) {
829
+ spyOn(console, 'error');
830
+
831
+ var defaultCreateElement = document.createElement.bind(document);
832
+
833
+ try {
834
+ document.createElement = element => {
835
+ var container = defaultCreateElement(element);
836
+ container.shadyRoot = {};
837
+ return container;
838
+ };
839
+
840
+ mountComponent({is: 'custom-shady-div'});
841
+ expect(console.error.calls.count()).toBe(1);
842
+ expect(console.error.calls.argsFor(0)[0]).toContain(
843
+ 'A component is using shady DOM. Using shady DOM with React can ' +
844
+ 'cause things to break subtly.'
845
+ );
846
+
847
+ mountComponent({is: 'custom-shady-div2'});
848
+ expect(console.error.calls.count()).toBe(1);
849
+
850
+ } finally {
851
+ document.createElement = defaultCreateElement;
852
+ }
853
+ }
854
+ });
855
+
856
it('should treat menuitem as a void element but still create the closing tag', () => {
857
var container = document.createElement('div');
858