Skip to content

Commit

Permalink
Deduplicate "unknown prop" warning to ease pain of version updates
Browse files Browse the repository at this point in the history
Some widely used libraries which depend on React have used syntax that
ends up passing non-standard props to default React components like
'dom' and 'div'. For example:
```
  // where this.props contains attributes like 'foo'
  return <div {...this.props} />;
```

Since we started throwing warnings for this syntax in Reactv15.2 some
libraries have not upgraded past v15.2.

Now, because of deprecations in 15.5, some libraries will only work with
Reactv15.5, and this causes a conflict when other libraries are pinned
at v15.1. The overall effect is that either there are version conflicts
or the "unknown prop" warning is thrown all over the place, and users of
these libraries can't fix the warnings.

See facebook#9466 for more context.

We are deduping this warning in hopes that it allows more library
authors to update to the latest version of React. React v16.0 may take a
different approach to this issue. For some related discussion, see
facebook#9477

**Test Plan:**
 - Added a unit test
 - Manually tested with a fixture that was not committed; https://gist.github.com/flarnie/db011bf54206f30b9983cd4dc674c82e
  • Loading branch information
flarnie committed Apr 21, 2017
1 parent 1926f9a commit 4f0abcb
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
15 changes: 15 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,21 @@ describe('ReactDOMComponent', () => {
);
});

it('should only warn for the first unknown props passed', () => {
spyOn(console, 'error');
var container = document.createElement('div');
ReactDOM.render(
// the 'span' should not trigger a second warning in React v15.6.0
<div foo="bar" baz="qux"><span hello="world" /></div>,
container,
);
expect(console.error.calls.count(0)).toBe(1);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Unknown props `foo`, `baz` on <div> tag. Remove these props from the element. ' +
'For details, see https://fb.me/react-unknown-prop\n in div (at **)'
);
});

it('should warn for onDblClick prop', () => {
spyOn(console, 'error');
var container = document.createElement('div');
Expand Down
8 changes: 8 additions & 0 deletions src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,13 @@ if (__DEV__) {
};
}

let alreadyWarned = false;

var warnUnknownProperties = function(debugID, element) {
if (alreadyWarned) {
return;
}

var unknownProps = [];
for (var key in element.props) {
var isValid = validateProperty(element.type, key, debugID);
Expand All @@ -117,6 +123,7 @@ var warnUnknownProperties = function(debugID, element) {
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
);
alreadyWarned = true;
} else if (unknownProps.length > 1) {
warning(
false,
Expand All @@ -126,6 +133,7 @@ var warnUnknownProperties = function(debugID, element) {
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
);
alreadyWarned = true;
}
};

Expand Down

0 comments on commit 4f0abcb

Please sign in to comment.