Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Improve warning message, remove dedup check, and unmock console.error
**what is the change?:**
Various changes to get this closer to being finished;
- Improved warning message (thanks @spicyj!!!)
- Removed dedup check on warning
- Remove mocking of 'console.error' in portals test

**why make this change?:**
- warning message improvement: communicates better with users
- Remove dedup check: it wasn't important in this case
- Remove mocking of 'console.error'; we don't want to ignore an
  inaccurate warning, even for an "unstable" feature.

**test plan:**
`yarn test` -> follow-up commits will fix the remaining tests

**issue:**
issue facebook#8854
  • Loading branch information
flarnie committed Jul 20, 2017
1 parent 5a7da89 commit cfd3c35
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 15 deletions.
13 changes: 5 additions & 8 deletions src/renderers/dom/fiber/ReactDOMFiberEntry.js
Expand Up @@ -531,21 +531,18 @@ function renderSubtreeIntoContainer(
);

if (__DEV__) {
let warned;
if (!warned
&& container._reactRootContainer
if (container._reactRootContainer
&& container.nodeType !== COMMENT_NODE) {
const firstChild = container.firstChild;
const hostInstance =
DOMRenderer.findHostInstance(container._reactRootContainer.current);
if (hostInstance) {
warned = true;
warning(
hostInstance.parentNode === container,
'render(...): It looks like the content of this container may have ' +
'been updated or cleared outside of React. ' +
'This can cause errors or failed updates to the container. ' +
'Please call `ReactDOM.render` with your new content.',
'render(...): It looks like the React-rendered content of this ' +
'container was removed without using React. This is not ' +
'supported and will cause errors. Instead, call ' +
'ReactDOM.unmountComponentAtNode to empty a container.',
);
}
}
Expand Down
7 changes: 0 additions & 7 deletions src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
Expand Up @@ -758,13 +758,6 @@ describe('ReactDOMFiber', () => {
});

it('should update portal context if it changes due to re-render', () => {
// We mock out console error because this causes a warning to fire;
// it looks as if the content should be in the 'container' div,
// and when it's not there after the first render React assumes the
// container was cleared outside of React.
// for now we are not concerned about this warning firing.
spyOn(console, 'error');

var portalContainer = document.createElement('div');

class Component extends React.Component {
Expand Down

0 comments on commit cfd3c35

Please sign in to comment.