2016-10-23 08:41:46 +08:00
|
|
|
/**
|
2017-09-25 04:48:13 +08:00
|
|
|
* Copyright (c) 2013-present, Facebook, Inc.
|
2016-10-23 08:41:46 +08:00
|
|
|
*
|
2017-09-25 04:48:13 +08:00
|
|
|
* This source code is licensed under the MIT license found in the
|
|
|
|
|
* LICENSE file in the root directory of this source tree.
|
2016-10-23 08:41:46 +08:00
|
|
|
*
|
|
|
|
|
* @flow
|
|
|
|
|
*/
|
|
|
|
|
|
2017-10-25 07:55:00 +08:00
|
|
|
import type {Fiber} from 'react-reconciler/src/ReactFiber';
|
2016-10-23 08:41:46 +08:00
|
|
|
|
2017-11-03 03:50:03 +08:00
|
|
|
import invariant from 'fbjs/lib/invariant';
|
|
|
|
|
import warning from 'fbjs/lib/warning';
|
2017-10-26 02:07:54 +08:00
|
|
|
|
2017-11-03 03:50:03 +08:00
|
|
|
import * as ReactInstanceMap from './ReactInstanceMap';
|
|
|
|
|
import {ReactCurrentOwner} from './ReactGlobalSharedState';
|
|
|
|
|
import getComponentName from './getComponentName';
|
|
|
|
|
import {
|
Add warning for rendering into container that was updated manually (#10210)
* RFC Add warning for rendering into container that was updated manually
RFC because we still need to tidy this up and verify that all tests
pass.
**what is the change?:**
We want to warn when users render into a container which was manually
emptied or updated outside of React. This can lead to the cryptic error
about not being able to remove a node, or just lead to silent failures
of render. This warning should make things more clear.
Note that this covers the case where the contents of the root container
are manually updated, but does not cover the case where something was
manually updated deeper in the tree.
**why make this change?:**
To maintain parity and increase clarity before releasing v16.0 beta.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
last item under the '16 beta' checklist.
* Add test and tweak check for rendering into manually updated container
STILL TODO: figure out how to skip this warning when the component
renders to a portal.
Unfortunately 'ReactPortal.isPortal(children)' returns false, even in
the failing test where we are rendering to a portal.
**what is the change?:**
- added a test for the case where we call 'ReactDOM.render' with a new
container, using a key or a different type, after the contents of the
first container were messed with outside of React. This case throws,
and now at least there will be an informative warning along with the
error.
- We updated the check to compare the parent of the 'hostInstance' to
the container; this seems less fragile
- tweaked some comments
**why make this change?:**
Continue improving this to make it more final.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Stub our `console.error` in one of the portal tests
**what is the change?:**
See title
**why make this change?:**
See comment in the code
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Skip warning in 'ReactDOMFiberEntry' when mounting to Comment node
**what is the change?:**
We have a warning for cases when the container doesn't match the parent
which we remembered the previously rendered content being rendered into.
We are skipping that warning when you render into a 'comment' node.
**why make this change?:**
Basically, if you render into a 'comment' node, then the parent of the
comment node is the container for your rendered content. We could check
for similarity there but rendering into a comment node seems like a
corner case and I'd rather skip the warning without knowing more about
what could happen in that case.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* 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 #8854
* Possible fix for issue of incorrect warning for portal re-render
**what is the change?:**
Add a property to a container which was rendered into using
`ReactDOM.unstable_createPortal`.
**why make this change?:**
We don't want to warn for mismatching container nodes in this case - the
user intentionally rendered into the portal container instead of the
original container.
concerns;
- will this affect React Native badly?
- will this add bloat to the portal code? seems small enough but not
sure.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fix logic for checking if the host instance container is a portal
**what is the change?:**
When focusing on fixing the warning to not check when we are using
portals, I missed checking for the existence of the host instance parent
before checking if it was a portal. This adds the missing null checks.
**why make this change?:**
To fix a bug that the previous commit introduced.
**test plan:**
`yarn test`
-> follow-up commits fix more of the test failures
**issue:**
https://github.com/facebook/react/issues/8854
* Clean up new tests in ReactDOMFiber-test
**what is the change?:**
- removed extra single quotes, downgrade double quotes to single
- update expected warning message to match latest warning message
- fix indentation
**why make this change?:**
- get tests passing
- code maintainability/readability
**test plan:**
`yarn test`
follow up commits will fix the remaining tests
**issue:**
https://github.com/facebook/react/issues/8854
* Add 'unmountComponentAtNode' call in test for reconciling pre-rendered markup
**what is the change?:**
We have a test that verifies React can reconcile text from pre-rendered
mark-up. It tests React doing this for three strings and three empty
strings.
This adds a call to 'unmountComponentAtNode' between the two
expectations for strings and empty strings.
**why make this change?:**
We now warn when someone messes with the DOM inside of a node in such a
way that removes the React-rendered content. This test was doing that. I
can't think of a situation where this would happen with server-side
rendering without the need to call 'unmountComponentAtNode' before
inserting the server-side rendered content.
**test plan:**
`yarn test`
Only one more failing test, will fix that in the next commit.
**issue:**
https://github.com/facebook/react/issues/8854
* ran prettier
* remove unused variable
* run scripts/fiber/record-tests
* Fix type error and improve name of portal container flag
**NOTE:** I am still looking for a good place to move this flag
assignment to, or a better approach. This does some intermediate fixes.
**what is the change?:**
- fixed flow error by allowing optional flag on a DOMContainer that
indicates it was used as a portal container.
- renamed the flag to something which makes more sense
**why make this change?:**
- get Flow passing
- make this change make more sense
We are still not sure about adding this flag; a follow-up diff may move
it or take a different approach.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Add flag to portalContainer on mount instead of in `createPortal`
**what is the change?:**
We add a flag to the container of a 'portal' in the 'commit work' phase
in Fiber. This is right before we call `appendChildToContainer`.
**why make this change?:**
- Sometimes people call `ReactDOM.render(... container)`, then manually
clear the content of the `container`, and then try to call another
`ReactDOM.render(... container)`.
- This leads to cryptic errors or silent failure because we hold a
reference to the node that was rendered the first time, and expect it
to still be inside the container.
- We added a warning for this issue in `renderSubtreeIntoContainer`, but
when a component renders something returned by
`ReactDOM.unstable_createPortal(<Component />, portalContainer);`,
then the child is inside the `portalContainer` and not the `container,
but that is valid and we want to skip warning in that case.
Inside `renderSubtreeIntoContainer` we don't have the info to determine
if a child was rendered into a `portalContainer` or a `container`, and
adding this flag lets us figure that out and skip the warning.
We originally added the flag in the call to
`ReactDOM.unstable_createPortal` but that seemed like a method that
should be "pure" and free of side-effects. This commit moves the
flag-adding to happen when we mount the portal component.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Force an 'any' type for the `hostInstance.parentNode` in warning check
**what is the change?:**
This is awful. :(
I'm not sure how else to let Flow know that we expect that this might be
a sort of `DOMContainer` type and not just a normal `Node` type.
To at least make the type information clear we added a comment.
**why make this change?:**
To get `flow` passing. Looks like we have `any` types sprinkled
throughout this file. phooey. :(
**test plan:**
`yarn flow`
**issue:**
https://github.com/facebook/react/issues/8854
* Ignore portals in `DOMRenderer.findHostInstance`
**what is the change?:**
We want to ignore portals when firing a certain warning.
This allows us to get the host instance and ignore portals.
Also added a new snapshot recording while fixing things.
**why make this change?:**
Originally we had added a flag to the DOM node which was used for
rendering the portal, and then could notice and ignore children rendered
into those nodes.
However, it's better to just ignore portals in
`DOMRenderer.findHostInstance` because
- we will not ignore a non-portal second child with this approach
- we meant to ignore portals in this method anyway (according to a
'TODO' comment)
- this change only affects the DOM renderer, instead of changing code
which is shared with RN and other renderers
- we avoid adding unneeded expandos
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Ran prettier
* Remove error snapshot test
I think there is a bug where an empty snapshot is treated as an 'outdated snapshot'.
If I delete the obsolute snapshot, and run ReactDOMFiber-test.js it generates a new snapshot.
But then when I run the test with the newly generated snapshot, it says "1 obsolete snapshot found",
At some point I will file an issue with Jest. For now going to skip the snapshot generation for the error message in the new test.
* Remove expando that we were adding to portal container
**what is the change?:**
see title
**why make this change?:**
this is part of an old approach to detecting portals, and we have
instead added a check in the `findHostInstance` method to filter out
portals.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fork `findHostInstance` to make `findHostInstanceWithNoPortals`
**what is the change?:**
We need to get host instances, but filter out portals. There is not
currently a method for that.
**why make this change?:**
Rather than change the existing `findHostInstance` method, which would
affect the behavior of the public `findDOMNode` method, we are forking.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
2017-07-21 23:48:46 +08:00
|
|
|
ClassComponent,
|
2016-10-23 08:41:46 +08:00
|
|
|
HostComponent,
|
Add warning for rendering into container that was updated manually (#10210)
* RFC Add warning for rendering into container that was updated manually
RFC because we still need to tidy this up and verify that all tests
pass.
**what is the change?:**
We want to warn when users render into a container which was manually
emptied or updated outside of React. This can lead to the cryptic error
about not being able to remove a node, or just lead to silent failures
of render. This warning should make things more clear.
Note that this covers the case where the contents of the root container
are manually updated, but does not cover the case where something was
manually updated deeper in the tree.
**why make this change?:**
To maintain parity and increase clarity before releasing v16.0 beta.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
last item under the '16 beta' checklist.
* Add test and tweak check for rendering into manually updated container
STILL TODO: figure out how to skip this warning when the component
renders to a portal.
Unfortunately 'ReactPortal.isPortal(children)' returns false, even in
the failing test where we are rendering to a portal.
**what is the change?:**
- added a test for the case where we call 'ReactDOM.render' with a new
container, using a key or a different type, after the contents of the
first container were messed with outside of React. This case throws,
and now at least there will be an informative warning along with the
error.
- We updated the check to compare the parent of the 'hostInstance' to
the container; this seems less fragile
- tweaked some comments
**why make this change?:**
Continue improving this to make it more final.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Stub our `console.error` in one of the portal tests
**what is the change?:**
See title
**why make this change?:**
See comment in the code
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Skip warning in 'ReactDOMFiberEntry' when mounting to Comment node
**what is the change?:**
We have a warning for cases when the container doesn't match the parent
which we remembered the previously rendered content being rendered into.
We are skipping that warning when you render into a 'comment' node.
**why make this change?:**
Basically, if you render into a 'comment' node, then the parent of the
comment node is the container for your rendered content. We could check
for similarity there but rendering into a comment node seems like a
corner case and I'd rather skip the warning without knowing more about
what could happen in that case.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* 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 #8854
* Possible fix for issue of incorrect warning for portal re-render
**what is the change?:**
Add a property to a container which was rendered into using
`ReactDOM.unstable_createPortal`.
**why make this change?:**
We don't want to warn for mismatching container nodes in this case - the
user intentionally rendered into the portal container instead of the
original container.
concerns;
- will this affect React Native badly?
- will this add bloat to the portal code? seems small enough but not
sure.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fix logic for checking if the host instance container is a portal
**what is the change?:**
When focusing on fixing the warning to not check when we are using
portals, I missed checking for the existence of the host instance parent
before checking if it was a portal. This adds the missing null checks.
**why make this change?:**
To fix a bug that the previous commit introduced.
**test plan:**
`yarn test`
-> follow-up commits fix more of the test failures
**issue:**
https://github.com/facebook/react/issues/8854
* Clean up new tests in ReactDOMFiber-test
**what is the change?:**
- removed extra single quotes, downgrade double quotes to single
- update expected warning message to match latest warning message
- fix indentation
**why make this change?:**
- get tests passing
- code maintainability/readability
**test plan:**
`yarn test`
follow up commits will fix the remaining tests
**issue:**
https://github.com/facebook/react/issues/8854
* Add 'unmountComponentAtNode' call in test for reconciling pre-rendered markup
**what is the change?:**
We have a test that verifies React can reconcile text from pre-rendered
mark-up. It tests React doing this for three strings and three empty
strings.
This adds a call to 'unmountComponentAtNode' between the two
expectations for strings and empty strings.
**why make this change?:**
We now warn when someone messes with the DOM inside of a node in such a
way that removes the React-rendered content. This test was doing that. I
can't think of a situation where this would happen with server-side
rendering without the need to call 'unmountComponentAtNode' before
inserting the server-side rendered content.
**test plan:**
`yarn test`
Only one more failing test, will fix that in the next commit.
**issue:**
https://github.com/facebook/react/issues/8854
* ran prettier
* remove unused variable
* run scripts/fiber/record-tests
* Fix type error and improve name of portal container flag
**NOTE:** I am still looking for a good place to move this flag
assignment to, or a better approach. This does some intermediate fixes.
**what is the change?:**
- fixed flow error by allowing optional flag on a DOMContainer that
indicates it was used as a portal container.
- renamed the flag to something which makes more sense
**why make this change?:**
- get Flow passing
- make this change make more sense
We are still not sure about adding this flag; a follow-up diff may move
it or take a different approach.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Add flag to portalContainer on mount instead of in `createPortal`
**what is the change?:**
We add a flag to the container of a 'portal' in the 'commit work' phase
in Fiber. This is right before we call `appendChildToContainer`.
**why make this change?:**
- Sometimes people call `ReactDOM.render(... container)`, then manually
clear the content of the `container`, and then try to call another
`ReactDOM.render(... container)`.
- This leads to cryptic errors or silent failure because we hold a
reference to the node that was rendered the first time, and expect it
to still be inside the container.
- We added a warning for this issue in `renderSubtreeIntoContainer`, but
when a component renders something returned by
`ReactDOM.unstable_createPortal(<Component />, portalContainer);`,
then the child is inside the `portalContainer` and not the `container,
but that is valid and we want to skip warning in that case.
Inside `renderSubtreeIntoContainer` we don't have the info to determine
if a child was rendered into a `portalContainer` or a `container`, and
adding this flag lets us figure that out and skip the warning.
We originally added the flag in the call to
`ReactDOM.unstable_createPortal` but that seemed like a method that
should be "pure" and free of side-effects. This commit moves the
flag-adding to happen when we mount the portal component.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Force an 'any' type for the `hostInstance.parentNode` in warning check
**what is the change?:**
This is awful. :(
I'm not sure how else to let Flow know that we expect that this might be
a sort of `DOMContainer` type and not just a normal `Node` type.
To at least make the type information clear we added a comment.
**why make this change?:**
To get `flow` passing. Looks like we have `any` types sprinkled
throughout this file. phooey. :(
**test plan:**
`yarn flow`
**issue:**
https://github.com/facebook/react/issues/8854
* Ignore portals in `DOMRenderer.findHostInstance`
**what is the change?:**
We want to ignore portals when firing a certain warning.
This allows us to get the host instance and ignore portals.
Also added a new snapshot recording while fixing things.
**why make this change?:**
Originally we had added a flag to the DOM node which was used for
rendering the portal, and then could notice and ignore children rendered
into those nodes.
However, it's better to just ignore portals in
`DOMRenderer.findHostInstance` because
- we will not ignore a non-portal second child with this approach
- we meant to ignore portals in this method anyway (according to a
'TODO' comment)
- this change only affects the DOM renderer, instead of changing code
which is shared with RN and other renderers
- we avoid adding unneeded expandos
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Ran prettier
* Remove error snapshot test
I think there is a bug where an empty snapshot is treated as an 'outdated snapshot'.
If I delete the obsolute snapshot, and run ReactDOMFiber-test.js it generates a new snapshot.
But then when I run the test with the newly generated snapshot, it says "1 obsolete snapshot found",
At some point I will file an issue with Jest. For now going to skip the snapshot generation for the error message in the new test.
* Remove expando that we were adding to portal container
**what is the change?:**
see title
**why make this change?:**
this is part of an old approach to detecting portals, and we have
instead added a check in the `findHostInstance` method to filter out
portals.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fork `findHostInstance` to make `findHostInstanceWithNoPortals`
**what is the change?:**
We need to get host instances, but filter out portals. There is not
currently a method for that.
**why make this change?:**
Rather than change the existing `findHostInstance` method, which would
affect the behavior of the public `findDOMNode` method, we are forking.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
2017-07-21 23:48:46 +08:00
|
|
|
HostRoot,
|
|
|
|
|
HostPortal,
|
2016-10-23 08:41:46 +08:00
|
|
|
HostText,
|
2017-11-03 03:50:03 +08:00
|
|
|
} from './ReactTypeOfWork';
|
|
|
|
|
import {NoEffect, Placement} from './ReactTypeOfSideEffect';
|
2017-10-26 02:07:54 +08:00
|
|
|
|
2016-11-22 01:13:26 +08:00
|
|
|
var MOUNTING = 1;
|
|
|
|
|
var MOUNTED = 2;
|
|
|
|
|
var UNMOUNTED = 3;
|
|
|
|
|
|
2017-03-14 08:05:18 +08:00
|
|
|
function isFiberMountedImpl(fiber: Fiber): number {
|
2016-10-25 07:02:22 +08:00
|
|
|
let node = fiber;
|
|
|
|
|
if (!fiber.alternate) {
|
2016-10-25 04:46:37 +08:00
|
|
|
// If there is no alternate, this might be a new tree that isn't inserted
|
|
|
|
|
// yet. If it is, then it will have a pending insertion effect on it.
|
|
|
|
|
if ((node.effectTag & Placement) !== NoEffect) {
|
2016-11-22 01:13:26 +08:00
|
|
|
return MOUNTING;
|
2016-10-25 04:46:37 +08:00
|
|
|
}
|
|
|
|
|
while (node.return) {
|
|
|
|
|
node = node.return;
|
|
|
|
|
if ((node.effectTag & Placement) !== NoEffect) {
|
2016-11-22 01:13:26 +08:00
|
|
|
return MOUNTING;
|
2016-10-25 04:46:37 +08:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
} else {
|
|
|
|
|
while (node.return) {
|
|
|
|
|
node = node.return;
|
|
|
|
|
}
|
|
|
|
|
}
|
2016-12-02 02:36:28 +08:00
|
|
|
if (node.tag === HostRoot) {
|
|
|
|
|
// TODO: Check if this was a nested HostRoot when used with
|
2016-10-25 04:46:37 +08:00
|
|
|
// renderContainerIntoSubtree.
|
2016-11-22 01:13:26 +08:00
|
|
|
return MOUNTED;
|
2016-10-25 04:46:37 +08:00
|
|
|
}
|
2016-11-22 01:13:26 +08:00
|
|
|
// If we didn't hit the root, that means that we're in an disconnected tree
|
|
|
|
|
// that has been unmounted.
|
|
|
|
|
return UNMOUNTED;
|
2016-10-25 07:02:22 +08:00
|
|
|
}
|
2017-11-03 03:50:03 +08:00
|
|
|
|
|
|
|
|
export function isFiberMounted(fiber: Fiber): boolean {
|
2016-12-01 04:04:00 +08:00
|
|
|
return isFiberMountedImpl(fiber) === MOUNTED;
|
2017-11-03 03:50:03 +08:00
|
|
|
}
|
2016-10-25 07:02:22 +08:00
|
|
|
|
2017-11-03 03:50:03 +08:00
|
|
|
export function isMounted(component: React$Component<any, any>): boolean {
|
2017-02-08 03:48:45 +08:00
|
|
|
if (__DEV__) {
|
2017-03-14 08:05:18 +08:00
|
|
|
const owner = (ReactCurrentOwner.current: any);
|
2017-02-08 03:48:45 +08:00
|
|
|
if (owner !== null && owner.tag === ClassComponent) {
|
2017-03-14 08:05:18 +08:00
|
|
|
const ownerFiber: Fiber = owner;
|
2017-02-08 03:48:45 +08:00
|
|
|
const instance = ownerFiber.stateNode;
|
|
|
|
|
warning(
|
|
|
|
|
instance._warnedAboutRefsInRender,
|
|
|
|
|
'%s is accessing isMounted inside its render() function. ' +
|
2017-03-14 08:05:18 +08:00
|
|
|
'render() should be a pure function of props and state. It should ' +
|
|
|
|
|
'never access something that requires stale data from the previous ' +
|
|
|
|
|
'render, such as refs. Move this logic to componentDidMount and ' +
|
|
|
|
|
'componentDidUpdate instead.',
|
|
|
|
|
getComponentName(ownerFiber) || 'A component',
|
2017-02-08 03:48:45 +08:00
|
|
|
);
|
|
|
|
|
instance._warnedAboutRefsInRender = true;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2017-03-14 08:05:18 +08:00
|
|
|
var fiber: ?Fiber = ReactInstanceMap.get(component);
|
2016-10-25 07:02:22 +08:00
|
|
|
if (!fiber) {
|
|
|
|
|
return false;
|
|
|
|
|
}
|
2016-12-01 04:04:00 +08:00
|
|
|
return isFiberMountedImpl(fiber) === MOUNTED;
|
2017-11-03 03:50:03 +08:00
|
|
|
}
|
2016-10-23 09:35:07 +08:00
|
|
|
|
2016-12-02 13:21:05 +08:00
|
|
|
function assertIsMounted(fiber) {
|
|
|
|
|
invariant(
|
|
|
|
|
isFiberMountedImpl(fiber) === MOUNTED,
|
2017-03-14 08:05:18 +08:00
|
|
|
'Unable to find node on an unmounted component.',
|
2016-12-02 13:21:05 +08:00
|
|
|
);
|
|
|
|
|
}
|
|
|
|
|
|
2017-11-03 03:50:03 +08:00
|
|
|
export function findCurrentFiberUsingSlowPath(fiber: Fiber): Fiber | null {
|
2016-12-02 13:21:05 +08:00
|
|
|
let alternate = fiber.alternate;
|
|
|
|
|
if (!alternate) {
|
|
|
|
|
// If there is no alternate, then we only need to check if it is mounted.
|
|
|
|
|
const state = isFiberMountedImpl(fiber);
|
2016-11-22 01:13:26 +08:00
|
|
|
invariant(
|
2016-12-02 13:21:05 +08:00
|
|
|
state !== UNMOUNTED,
|
2017-03-14 08:05:18 +08:00
|
|
|
'Unable to find node on an unmounted component.',
|
2016-11-22 01:13:26 +08:00
|
|
|
);
|
2016-12-02 13:21:05 +08:00
|
|
|
if (state === MOUNTING) {
|
|
|
|
|
return null;
|
|
|
|
|
}
|
|
|
|
|
return fiber;
|
2016-10-25 07:02:22 +08:00
|
|
|
}
|
2016-12-02 13:21:05 +08:00
|
|
|
// If we have two possible branches, we'll walk backwards up to the root
|
|
|
|
|
// to see what path the root points to. On the way we may hit one of the
|
|
|
|
|
// special cases and we'll deal with them.
|
|
|
|
|
let a = fiber;
|
|
|
|
|
let b = alternate;
|
|
|
|
|
while (true) {
|
|
|
|
|
let parentA = a.return;
|
2017-01-31 10:41:25 +08:00
|
|
|
let parentB = parentA ? parentA.alternate : null;
|
2016-12-02 13:21:05 +08:00
|
|
|
if (!parentA || !parentB) {
|
|
|
|
|
// We're at the root.
|
|
|
|
|
break;
|
|
|
|
|
}
|
2017-01-31 10:41:25 +08:00
|
|
|
|
|
|
|
|
// If both copies of the parent fiber point to the same child, we can
|
|
|
|
|
// assume that the child is current. This happens when we bailout on low
|
|
|
|
|
// priority: the bailed out fiber's child reuses the current child.
|
2016-12-02 13:21:05 +08:00
|
|
|
if (parentA.child === parentB.child) {
|
|
|
|
|
let child = parentA.child;
|
|
|
|
|
while (child) {
|
|
|
|
|
if (child === a) {
|
|
|
|
|
// We've determined that A is the current branch.
|
|
|
|
|
assertIsMounted(parentA);
|
|
|
|
|
return fiber;
|
|
|
|
|
}
|
|
|
|
|
if (child === b) {
|
|
|
|
|
// We've determined that B is the current branch.
|
|
|
|
|
assertIsMounted(parentA);
|
|
|
|
|
return alternate;
|
|
|
|
|
}
|
|
|
|
|
child = child.sibling;
|
|
|
|
|
}
|
|
|
|
|
// We should never have an alternate for any mounting node. So the only
|
|
|
|
|
// way this could possibly happen is if this was unmounted, if at all.
|
2017-03-14 08:05:18 +08:00
|
|
|
invariant(false, 'Unable to find node on an unmounted component.');
|
2016-12-02 13:21:05 +08:00
|
|
|
}
|
2017-01-31 10:41:25 +08:00
|
|
|
|
|
|
|
|
if (a.return !== b.return) {
|
|
|
|
|
// The return pointer of A and the return pointer of B point to different
|
|
|
|
|
// fibers. We assume that return pointers never criss-cross, so A must
|
|
|
|
|
// belong to the child set of A.return, and B must belong to the child
|
|
|
|
|
// set of B.return.
|
|
|
|
|
a = parentA;
|
|
|
|
|
b = parentB;
|
|
|
|
|
} else {
|
2017-06-28 00:53:16 +08:00
|
|
|
// The return pointers point to the same fiber. We'll have to use the
|
2017-01-31 10:41:25 +08:00
|
|
|
// default, slow path: scan the child sets of each parent alternate to see
|
|
|
|
|
// which child belongs to which set.
|
|
|
|
|
//
|
|
|
|
|
// Search parent A's child set
|
|
|
|
|
let didFindChild = false;
|
|
|
|
|
let child = parentA.child;
|
|
|
|
|
while (child) {
|
|
|
|
|
if (child === a) {
|
|
|
|
|
didFindChild = true;
|
|
|
|
|
a = parentA;
|
|
|
|
|
b = parentB;
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
if (child === b) {
|
|
|
|
|
didFindChild = true;
|
|
|
|
|
b = parentA;
|
|
|
|
|
a = parentB;
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
child = child.sibling;
|
|
|
|
|
}
|
|
|
|
|
if (!didFindChild) {
|
|
|
|
|
// Search parent B's child set
|
|
|
|
|
child = parentB.child;
|
|
|
|
|
while (child) {
|
|
|
|
|
if (child === a) {
|
2017-02-01 03:35:10 +08:00
|
|
|
didFindChild = true;
|
2017-01-31 10:41:25 +08:00
|
|
|
a = parentB;
|
|
|
|
|
b = parentA;
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
if (child === b) {
|
2017-02-01 03:35:10 +08:00
|
|
|
didFindChild = true;
|
2017-01-31 10:41:25 +08:00
|
|
|
b = parentB;
|
|
|
|
|
a = parentA;
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
child = child.sibling;
|
|
|
|
|
}
|
2017-02-01 03:35:10 +08:00
|
|
|
invariant(
|
|
|
|
|
didFindChild,
|
|
|
|
|
'Child was not found in either parent set. This indicates a bug ' +
|
2017-07-27 02:08:22 +08:00
|
|
|
'in React related to the return pointer. Please file an issue.',
|
2017-02-01 03:35:10 +08:00
|
|
|
);
|
2017-01-31 10:41:25 +08:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2016-12-02 13:21:05 +08:00
|
|
|
invariant(
|
|
|
|
|
a.alternate === b,
|
2017-07-27 02:08:22 +08:00
|
|
|
"Return fibers should always be each others' alternates. " +
|
|
|
|
|
'This error is likely caused by a bug in React. Please file an issue.',
|
2016-12-02 13:21:05 +08:00
|
|
|
);
|
|
|
|
|
}
|
|
|
|
|
// If the root is not a host container, we're in a disconnected tree. I.e.
|
|
|
|
|
// unmounted.
|
|
|
|
|
invariant(
|
|
|
|
|
a.tag === HostRoot,
|
2017-03-14 08:05:18 +08:00
|
|
|
'Unable to find node on an unmounted component.',
|
2016-12-02 13:21:05 +08:00
|
|
|
);
|
|
|
|
|
if (a.stateNode.current === a) {
|
|
|
|
|
// We've determined that A is the current branch.
|
|
|
|
|
return fiber;
|
|
|
|
|
}
|
|
|
|
|
// Otherwise B has to be current branch.
|
|
|
|
|
return alternate;
|
|
|
|
|
}
|
2016-10-25 07:02:22 +08:00
|
|
|
|
2017-11-03 03:50:03 +08:00
|
|
|
export function findCurrentHostFiber(parent: Fiber): Fiber | null {
|
2016-12-02 13:21:05 +08:00
|
|
|
const currentParent = findCurrentFiberUsingSlowPath(parent);
|
|
|
|
|
if (!currentParent) {
|
|
|
|
|
return null;
|
2016-11-21 04:27:09 +08:00
|
|
|
}
|
|
|
|
|
|
2016-10-25 07:02:22 +08:00
|
|
|
// Next we'll drill down this component to find the first HostComponent/Text.
|
2017-03-14 08:05:18 +08:00
|
|
|
let node: Fiber = currentParent;
|
2016-10-23 08:41:46 +08:00
|
|
|
while (true) {
|
|
|
|
|
if (node.tag === HostComponent || node.tag === HostText) {
|
|
|
|
|
return node;
|
|
|
|
|
} else if (node.child) {
|
Add warning for rendering into container that was updated manually (#10210)
* RFC Add warning for rendering into container that was updated manually
RFC because we still need to tidy this up and verify that all tests
pass.
**what is the change?:**
We want to warn when users render into a container which was manually
emptied or updated outside of React. This can lead to the cryptic error
about not being able to remove a node, or just lead to silent failures
of render. This warning should make things more clear.
Note that this covers the case where the contents of the root container
are manually updated, but does not cover the case where something was
manually updated deeper in the tree.
**why make this change?:**
To maintain parity and increase clarity before releasing v16.0 beta.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
last item under the '16 beta' checklist.
* Add test and tweak check for rendering into manually updated container
STILL TODO: figure out how to skip this warning when the component
renders to a portal.
Unfortunately 'ReactPortal.isPortal(children)' returns false, even in
the failing test where we are rendering to a portal.
**what is the change?:**
- added a test for the case where we call 'ReactDOM.render' with a new
container, using a key or a different type, after the contents of the
first container were messed with outside of React. This case throws,
and now at least there will be an informative warning along with the
error.
- We updated the check to compare the parent of the 'hostInstance' to
the container; this seems less fragile
- tweaked some comments
**why make this change?:**
Continue improving this to make it more final.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Stub our `console.error` in one of the portal tests
**what is the change?:**
See title
**why make this change?:**
See comment in the code
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Skip warning in 'ReactDOMFiberEntry' when mounting to Comment node
**what is the change?:**
We have a warning for cases when the container doesn't match the parent
which we remembered the previously rendered content being rendered into.
We are skipping that warning when you render into a 'comment' node.
**why make this change?:**
Basically, if you render into a 'comment' node, then the parent of the
comment node is the container for your rendered content. We could check
for similarity there but rendering into a comment node seems like a
corner case and I'd rather skip the warning without knowing more about
what could happen in that case.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* 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 #8854
* Possible fix for issue of incorrect warning for portal re-render
**what is the change?:**
Add a property to a container which was rendered into using
`ReactDOM.unstable_createPortal`.
**why make this change?:**
We don't want to warn for mismatching container nodes in this case - the
user intentionally rendered into the portal container instead of the
original container.
concerns;
- will this affect React Native badly?
- will this add bloat to the portal code? seems small enough but not
sure.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fix logic for checking if the host instance container is a portal
**what is the change?:**
When focusing on fixing the warning to not check when we are using
portals, I missed checking for the existence of the host instance parent
before checking if it was a portal. This adds the missing null checks.
**why make this change?:**
To fix a bug that the previous commit introduced.
**test plan:**
`yarn test`
-> follow-up commits fix more of the test failures
**issue:**
https://github.com/facebook/react/issues/8854
* Clean up new tests in ReactDOMFiber-test
**what is the change?:**
- removed extra single quotes, downgrade double quotes to single
- update expected warning message to match latest warning message
- fix indentation
**why make this change?:**
- get tests passing
- code maintainability/readability
**test plan:**
`yarn test`
follow up commits will fix the remaining tests
**issue:**
https://github.com/facebook/react/issues/8854
* Add 'unmountComponentAtNode' call in test for reconciling pre-rendered markup
**what is the change?:**
We have a test that verifies React can reconcile text from pre-rendered
mark-up. It tests React doing this for three strings and three empty
strings.
This adds a call to 'unmountComponentAtNode' between the two
expectations for strings and empty strings.
**why make this change?:**
We now warn when someone messes with the DOM inside of a node in such a
way that removes the React-rendered content. This test was doing that. I
can't think of a situation where this would happen with server-side
rendering without the need to call 'unmountComponentAtNode' before
inserting the server-side rendered content.
**test plan:**
`yarn test`
Only one more failing test, will fix that in the next commit.
**issue:**
https://github.com/facebook/react/issues/8854
* ran prettier
* remove unused variable
* run scripts/fiber/record-tests
* Fix type error and improve name of portal container flag
**NOTE:** I am still looking for a good place to move this flag
assignment to, or a better approach. This does some intermediate fixes.
**what is the change?:**
- fixed flow error by allowing optional flag on a DOMContainer that
indicates it was used as a portal container.
- renamed the flag to something which makes more sense
**why make this change?:**
- get Flow passing
- make this change make more sense
We are still not sure about adding this flag; a follow-up diff may move
it or take a different approach.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Add flag to portalContainer on mount instead of in `createPortal`
**what is the change?:**
We add a flag to the container of a 'portal' in the 'commit work' phase
in Fiber. This is right before we call `appendChildToContainer`.
**why make this change?:**
- Sometimes people call `ReactDOM.render(... container)`, then manually
clear the content of the `container`, and then try to call another
`ReactDOM.render(... container)`.
- This leads to cryptic errors or silent failure because we hold a
reference to the node that was rendered the first time, and expect it
to still be inside the container.
- We added a warning for this issue in `renderSubtreeIntoContainer`, but
when a component renders something returned by
`ReactDOM.unstable_createPortal(<Component />, portalContainer);`,
then the child is inside the `portalContainer` and not the `container,
but that is valid and we want to skip warning in that case.
Inside `renderSubtreeIntoContainer` we don't have the info to determine
if a child was rendered into a `portalContainer` or a `container`, and
adding this flag lets us figure that out and skip the warning.
We originally added the flag in the call to
`ReactDOM.unstable_createPortal` but that seemed like a method that
should be "pure" and free of side-effects. This commit moves the
flag-adding to happen when we mount the portal component.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Force an 'any' type for the `hostInstance.parentNode` in warning check
**what is the change?:**
This is awful. :(
I'm not sure how else to let Flow know that we expect that this might be
a sort of `DOMContainer` type and not just a normal `Node` type.
To at least make the type information clear we added a comment.
**why make this change?:**
To get `flow` passing. Looks like we have `any` types sprinkled
throughout this file. phooey. :(
**test plan:**
`yarn flow`
**issue:**
https://github.com/facebook/react/issues/8854
* Ignore portals in `DOMRenderer.findHostInstance`
**what is the change?:**
We want to ignore portals when firing a certain warning.
This allows us to get the host instance and ignore portals.
Also added a new snapshot recording while fixing things.
**why make this change?:**
Originally we had added a flag to the DOM node which was used for
rendering the portal, and then could notice and ignore children rendered
into those nodes.
However, it's better to just ignore portals in
`DOMRenderer.findHostInstance` because
- we will not ignore a non-portal second child with this approach
- we meant to ignore portals in this method anyway (according to a
'TODO' comment)
- this change only affects the DOM renderer, instead of changing code
which is shared with RN and other renderers
- we avoid adding unneeded expandos
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Ran prettier
* Remove error snapshot test
I think there is a bug where an empty snapshot is treated as an 'outdated snapshot'.
If I delete the obsolute snapshot, and run ReactDOMFiber-test.js it generates a new snapshot.
But then when I run the test with the newly generated snapshot, it says "1 obsolete snapshot found",
At some point I will file an issue with Jest. For now going to skip the snapshot generation for the error message in the new test.
* Remove expando that we were adding to portal container
**what is the change?:**
see title
**why make this change?:**
this is part of an old approach to detecting portals, and we have
instead added a check in the `findHostInstance` method to filter out
portals.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fork `findHostInstance` to make `findHostInstanceWithNoPortals`
**what is the change?:**
We need to get host instances, but filter out portals. There is not
currently a method for that.
**why make this change?:**
Rather than change the existing `findHostInstance` method, which would
affect the behavior of the public `findDOMNode` method, we are forking.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
2017-07-21 23:48:46 +08:00
|
|
|
node.child.return = node;
|
|
|
|
|
node = node.child;
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
if (node === currentParent) {
|
|
|
|
|
return null;
|
|
|
|
|
}
|
|
|
|
|
while (!node.sibling) {
|
|
|
|
|
if (!node.return || node.return === currentParent) {
|
|
|
|
|
return null;
|
|
|
|
|
}
|
|
|
|
|
node = node.return;
|
|
|
|
|
}
|
|
|
|
|
node.sibling.return = node.return;
|
|
|
|
|
node = node.sibling;
|
|
|
|
|
}
|
|
|
|
|
// Flow needs the return null here, but ESLint complains about it.
|
|
|
|
|
// eslint-disable-next-line no-unreachable
|
|
|
|
|
return null;
|
2017-11-03 03:50:03 +08:00
|
|
|
}
|
Add warning for rendering into container that was updated manually (#10210)
* RFC Add warning for rendering into container that was updated manually
RFC because we still need to tidy this up and verify that all tests
pass.
**what is the change?:**
We want to warn when users render into a container which was manually
emptied or updated outside of React. This can lead to the cryptic error
about not being able to remove a node, or just lead to silent failures
of render. This warning should make things more clear.
Note that this covers the case where the contents of the root container
are manually updated, but does not cover the case where something was
manually updated deeper in the tree.
**why make this change?:**
To maintain parity and increase clarity before releasing v16.0 beta.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
last item under the '16 beta' checklist.
* Add test and tweak check for rendering into manually updated container
STILL TODO: figure out how to skip this warning when the component
renders to a portal.
Unfortunately 'ReactPortal.isPortal(children)' returns false, even in
the failing test where we are rendering to a portal.
**what is the change?:**
- added a test for the case where we call 'ReactDOM.render' with a new
container, using a key or a different type, after the contents of the
first container were messed with outside of React. This case throws,
and now at least there will be an informative warning along with the
error.
- We updated the check to compare the parent of the 'hostInstance' to
the container; this seems less fragile
- tweaked some comments
**why make this change?:**
Continue improving this to make it more final.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Stub our `console.error` in one of the portal tests
**what is the change?:**
See title
**why make this change?:**
See comment in the code
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Skip warning in 'ReactDOMFiberEntry' when mounting to Comment node
**what is the change?:**
We have a warning for cases when the container doesn't match the parent
which we remembered the previously rendered content being rendered into.
We are skipping that warning when you render into a 'comment' node.
**why make this change?:**
Basically, if you render into a 'comment' node, then the parent of the
comment node is the container for your rendered content. We could check
for similarity there but rendering into a comment node seems like a
corner case and I'd rather skip the warning without knowing more about
what could happen in that case.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* 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 #8854
* Possible fix for issue of incorrect warning for portal re-render
**what is the change?:**
Add a property to a container which was rendered into using
`ReactDOM.unstable_createPortal`.
**why make this change?:**
We don't want to warn for mismatching container nodes in this case - the
user intentionally rendered into the portal container instead of the
original container.
concerns;
- will this affect React Native badly?
- will this add bloat to the portal code? seems small enough but not
sure.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fix logic for checking if the host instance container is a portal
**what is the change?:**
When focusing on fixing the warning to not check when we are using
portals, I missed checking for the existence of the host instance parent
before checking if it was a portal. This adds the missing null checks.
**why make this change?:**
To fix a bug that the previous commit introduced.
**test plan:**
`yarn test`
-> follow-up commits fix more of the test failures
**issue:**
https://github.com/facebook/react/issues/8854
* Clean up new tests in ReactDOMFiber-test
**what is the change?:**
- removed extra single quotes, downgrade double quotes to single
- update expected warning message to match latest warning message
- fix indentation
**why make this change?:**
- get tests passing
- code maintainability/readability
**test plan:**
`yarn test`
follow up commits will fix the remaining tests
**issue:**
https://github.com/facebook/react/issues/8854
* Add 'unmountComponentAtNode' call in test for reconciling pre-rendered markup
**what is the change?:**
We have a test that verifies React can reconcile text from pre-rendered
mark-up. It tests React doing this for three strings and three empty
strings.
This adds a call to 'unmountComponentAtNode' between the two
expectations for strings and empty strings.
**why make this change?:**
We now warn when someone messes with the DOM inside of a node in such a
way that removes the React-rendered content. This test was doing that. I
can't think of a situation where this would happen with server-side
rendering without the need to call 'unmountComponentAtNode' before
inserting the server-side rendered content.
**test plan:**
`yarn test`
Only one more failing test, will fix that in the next commit.
**issue:**
https://github.com/facebook/react/issues/8854
* ran prettier
* remove unused variable
* run scripts/fiber/record-tests
* Fix type error and improve name of portal container flag
**NOTE:** I am still looking for a good place to move this flag
assignment to, or a better approach. This does some intermediate fixes.
**what is the change?:**
- fixed flow error by allowing optional flag on a DOMContainer that
indicates it was used as a portal container.
- renamed the flag to something which makes more sense
**why make this change?:**
- get Flow passing
- make this change make more sense
We are still not sure about adding this flag; a follow-up diff may move
it or take a different approach.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Add flag to portalContainer on mount instead of in `createPortal`
**what is the change?:**
We add a flag to the container of a 'portal' in the 'commit work' phase
in Fiber. This is right before we call `appendChildToContainer`.
**why make this change?:**
- Sometimes people call `ReactDOM.render(... container)`, then manually
clear the content of the `container`, and then try to call another
`ReactDOM.render(... container)`.
- This leads to cryptic errors or silent failure because we hold a
reference to the node that was rendered the first time, and expect it
to still be inside the container.
- We added a warning for this issue in `renderSubtreeIntoContainer`, but
when a component renders something returned by
`ReactDOM.unstable_createPortal(<Component />, portalContainer);`,
then the child is inside the `portalContainer` and not the `container,
but that is valid and we want to skip warning in that case.
Inside `renderSubtreeIntoContainer` we don't have the info to determine
if a child was rendered into a `portalContainer` or a `container`, and
adding this flag lets us figure that out and skip the warning.
We originally added the flag in the call to
`ReactDOM.unstable_createPortal` but that seemed like a method that
should be "pure" and free of side-effects. This commit moves the
flag-adding to happen when we mount the portal component.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Force an 'any' type for the `hostInstance.parentNode` in warning check
**what is the change?:**
This is awful. :(
I'm not sure how else to let Flow know that we expect that this might be
a sort of `DOMContainer` type and not just a normal `Node` type.
To at least make the type information clear we added a comment.
**why make this change?:**
To get `flow` passing. Looks like we have `any` types sprinkled
throughout this file. phooey. :(
**test plan:**
`yarn flow`
**issue:**
https://github.com/facebook/react/issues/8854
* Ignore portals in `DOMRenderer.findHostInstance`
**what is the change?:**
We want to ignore portals when firing a certain warning.
This allows us to get the host instance and ignore portals.
Also added a new snapshot recording while fixing things.
**why make this change?:**
Originally we had added a flag to the DOM node which was used for
rendering the portal, and then could notice and ignore children rendered
into those nodes.
However, it's better to just ignore portals in
`DOMRenderer.findHostInstance` because
- we will not ignore a non-portal second child with this approach
- we meant to ignore portals in this method anyway (according to a
'TODO' comment)
- this change only affects the DOM renderer, instead of changing code
which is shared with RN and other renderers
- we avoid adding unneeded expandos
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Ran prettier
* Remove error snapshot test
I think there is a bug where an empty snapshot is treated as an 'outdated snapshot'.
If I delete the obsolute snapshot, and run ReactDOMFiber-test.js it generates a new snapshot.
But then when I run the test with the newly generated snapshot, it says "1 obsolete snapshot found",
At some point I will file an issue with Jest. For now going to skip the snapshot generation for the error message in the new test.
* Remove expando that we were adding to portal container
**what is the change?:**
see title
**why make this change?:**
this is part of an old approach to detecting portals, and we have
instead added a check in the `findHostInstance` method to filter out
portals.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fork `findHostInstance` to make `findHostInstanceWithNoPortals`
**what is the change?:**
We need to get host instances, but filter out portals. There is not
currently a method for that.
**why make this change?:**
Rather than change the existing `findHostInstance` method, which would
affect the behavior of the public `findDOMNode` method, we are forking.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
2017-07-21 23:48:46 +08:00
|
|
|
|
2017-11-03 03:50:03 +08:00
|
|
|
export function findCurrentHostFiberWithNoPortals(parent: Fiber): Fiber | null {
|
Add warning for rendering into container that was updated manually (#10210)
* RFC Add warning for rendering into container that was updated manually
RFC because we still need to tidy this up and verify that all tests
pass.
**what is the change?:**
We want to warn when users render into a container which was manually
emptied or updated outside of React. This can lead to the cryptic error
about not being able to remove a node, or just lead to silent failures
of render. This warning should make things more clear.
Note that this covers the case where the contents of the root container
are manually updated, but does not cover the case where something was
manually updated deeper in the tree.
**why make this change?:**
To maintain parity and increase clarity before releasing v16.0 beta.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
last item under the '16 beta' checklist.
* Add test and tweak check for rendering into manually updated container
STILL TODO: figure out how to skip this warning when the component
renders to a portal.
Unfortunately 'ReactPortal.isPortal(children)' returns false, even in
the failing test where we are rendering to a portal.
**what is the change?:**
- added a test for the case where we call 'ReactDOM.render' with a new
container, using a key or a different type, after the contents of the
first container were messed with outside of React. This case throws,
and now at least there will be an informative warning along with the
error.
- We updated the check to compare the parent of the 'hostInstance' to
the container; this seems less fragile
- tweaked some comments
**why make this change?:**
Continue improving this to make it more final.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Stub our `console.error` in one of the portal tests
**what is the change?:**
See title
**why make this change?:**
See comment in the code
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Skip warning in 'ReactDOMFiberEntry' when mounting to Comment node
**what is the change?:**
We have a warning for cases when the container doesn't match the parent
which we remembered the previously rendered content being rendered into.
We are skipping that warning when you render into a 'comment' node.
**why make this change?:**
Basically, if you render into a 'comment' node, then the parent of the
comment node is the container for your rendered content. We could check
for similarity there but rendering into a comment node seems like a
corner case and I'd rather skip the warning without knowing more about
what could happen in that case.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* 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 #8854
* Possible fix for issue of incorrect warning for portal re-render
**what is the change?:**
Add a property to a container which was rendered into using
`ReactDOM.unstable_createPortal`.
**why make this change?:**
We don't want to warn for mismatching container nodes in this case - the
user intentionally rendered into the portal container instead of the
original container.
concerns;
- will this affect React Native badly?
- will this add bloat to the portal code? seems small enough but not
sure.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fix logic for checking if the host instance container is a portal
**what is the change?:**
When focusing on fixing the warning to not check when we are using
portals, I missed checking for the existence of the host instance parent
before checking if it was a portal. This adds the missing null checks.
**why make this change?:**
To fix a bug that the previous commit introduced.
**test plan:**
`yarn test`
-> follow-up commits fix more of the test failures
**issue:**
https://github.com/facebook/react/issues/8854
* Clean up new tests in ReactDOMFiber-test
**what is the change?:**
- removed extra single quotes, downgrade double quotes to single
- update expected warning message to match latest warning message
- fix indentation
**why make this change?:**
- get tests passing
- code maintainability/readability
**test plan:**
`yarn test`
follow up commits will fix the remaining tests
**issue:**
https://github.com/facebook/react/issues/8854
* Add 'unmountComponentAtNode' call in test for reconciling pre-rendered markup
**what is the change?:**
We have a test that verifies React can reconcile text from pre-rendered
mark-up. It tests React doing this for three strings and three empty
strings.
This adds a call to 'unmountComponentAtNode' between the two
expectations for strings and empty strings.
**why make this change?:**
We now warn when someone messes with the DOM inside of a node in such a
way that removes the React-rendered content. This test was doing that. I
can't think of a situation where this would happen with server-side
rendering without the need to call 'unmountComponentAtNode' before
inserting the server-side rendered content.
**test plan:**
`yarn test`
Only one more failing test, will fix that in the next commit.
**issue:**
https://github.com/facebook/react/issues/8854
* ran prettier
* remove unused variable
* run scripts/fiber/record-tests
* Fix type error and improve name of portal container flag
**NOTE:** I am still looking for a good place to move this flag
assignment to, or a better approach. This does some intermediate fixes.
**what is the change?:**
- fixed flow error by allowing optional flag on a DOMContainer that
indicates it was used as a portal container.
- renamed the flag to something which makes more sense
**why make this change?:**
- get Flow passing
- make this change make more sense
We are still not sure about adding this flag; a follow-up diff may move
it or take a different approach.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Add flag to portalContainer on mount instead of in `createPortal`
**what is the change?:**
We add a flag to the container of a 'portal' in the 'commit work' phase
in Fiber. This is right before we call `appendChildToContainer`.
**why make this change?:**
- Sometimes people call `ReactDOM.render(... container)`, then manually
clear the content of the `container`, and then try to call another
`ReactDOM.render(... container)`.
- This leads to cryptic errors or silent failure because we hold a
reference to the node that was rendered the first time, and expect it
to still be inside the container.
- We added a warning for this issue in `renderSubtreeIntoContainer`, but
when a component renders something returned by
`ReactDOM.unstable_createPortal(<Component />, portalContainer);`,
then the child is inside the `portalContainer` and not the `container,
but that is valid and we want to skip warning in that case.
Inside `renderSubtreeIntoContainer` we don't have the info to determine
if a child was rendered into a `portalContainer` or a `container`, and
adding this flag lets us figure that out and skip the warning.
We originally added the flag in the call to
`ReactDOM.unstable_createPortal` but that seemed like a method that
should be "pure" and free of side-effects. This commit moves the
flag-adding to happen when we mount the portal component.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Force an 'any' type for the `hostInstance.parentNode` in warning check
**what is the change?:**
This is awful. :(
I'm not sure how else to let Flow know that we expect that this might be
a sort of `DOMContainer` type and not just a normal `Node` type.
To at least make the type information clear we added a comment.
**why make this change?:**
To get `flow` passing. Looks like we have `any` types sprinkled
throughout this file. phooey. :(
**test plan:**
`yarn flow`
**issue:**
https://github.com/facebook/react/issues/8854
* Ignore portals in `DOMRenderer.findHostInstance`
**what is the change?:**
We want to ignore portals when firing a certain warning.
This allows us to get the host instance and ignore portals.
Also added a new snapshot recording while fixing things.
**why make this change?:**
Originally we had added a flag to the DOM node which was used for
rendering the portal, and then could notice and ignore children rendered
into those nodes.
However, it's better to just ignore portals in
`DOMRenderer.findHostInstance` because
- we will not ignore a non-portal second child with this approach
- we meant to ignore portals in this method anyway (according to a
'TODO' comment)
- this change only affects the DOM renderer, instead of changing code
which is shared with RN and other renderers
- we avoid adding unneeded expandos
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Ran prettier
* Remove error snapshot test
I think there is a bug where an empty snapshot is treated as an 'outdated snapshot'.
If I delete the obsolute snapshot, and run ReactDOMFiber-test.js it generates a new snapshot.
But then when I run the test with the newly generated snapshot, it says "1 obsolete snapshot found",
At some point I will file an issue with Jest. For now going to skip the snapshot generation for the error message in the new test.
* Remove expando that we were adding to portal container
**what is the change?:**
see title
**why make this change?:**
this is part of an old approach to detecting portals, and we have
instead added a check in the `findHostInstance` method to filter out
portals.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
* Fork `findHostInstance` to make `findHostInstanceWithNoPortals`
**what is the change?:**
We need to get host instances, but filter out portals. There is not
currently a method for that.
**why make this change?:**
Rather than change the existing `findHostInstance` method, which would
affect the behavior of the public `findDOMNode` method, we are forking.
**test plan:**
`yarn test`
**issue:**
https://github.com/facebook/react/issues/8854
2017-07-21 23:48:46 +08:00
|
|
|
const currentParent = findCurrentFiberUsingSlowPath(parent);
|
|
|
|
|
if (!currentParent) {
|
|
|
|
|
return null;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// Next we'll drill down this component to find the first HostComponent/Text.
|
|
|
|
|
let node: Fiber = currentParent;
|
|
|
|
|
while (true) {
|
|
|
|
|
if (node.tag === HostComponent || node.tag === HostText) {
|
|
|
|
|
return node;
|
|
|
|
|
} else if (node.child && node.tag !== HostPortal) {
|
2016-12-02 13:21:05 +08:00
|
|
|
node.child.return = node;
|
2016-10-23 08:41:46 +08:00
|
|
|
node = node.child;
|
|
|
|
|
continue;
|
|
|
|
|
}
|
2016-12-02 13:21:05 +08:00
|
|
|
if (node === currentParent) {
|
2016-10-23 08:41:46 +08:00
|
|
|
return null;
|
|
|
|
|
}
|
|
|
|
|
while (!node.sibling) {
|
2016-12-02 13:21:05 +08:00
|
|
|
if (!node.return || node.return === currentParent) {
|
2016-10-23 08:41:46 +08:00
|
|
|
return null;
|
|
|
|
|
}
|
|
|
|
|
node = node.return;
|
|
|
|
|
}
|
2016-12-02 13:21:05 +08:00
|
|
|
node.sibling.return = node.return;
|
2016-10-23 08:41:46 +08:00
|
|
|
node = node.sibling;
|
|
|
|
|
}
|
2016-11-18 06:16:44 +08:00
|
|
|
// Flow needs the return null here, but ESLint complains about it.
|
|
|
|
|
// eslint-disable-next-line no-unreachable
|
2016-10-23 08:41:46 +08:00
|
|
|
return null;
|
2017-11-03 03:50:03 +08:00
|
|
|
}
|