[Fiber] Mount/unmount warnings and invariants #8919
Conversation
scripts/fiber/tests-failing.txt
Outdated
| * should work when measurement starts during reconciliation | ||
|
|
||
| src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js | ||
| * ignores null children |
There was a problem hiding this comment.
Why have they moved to failed?
There was a problem hiding this comment.
Just noticed that, not sure why
acdlite
force-pushed
the
fibermounttests
branch
from
787baab to
f09f632
Compare
|
@gaearon Ok fixed it |
| } | ||
|
|
||
| if (__DEV__) { | ||
| const isRootRenderedBySomeReact = Boolean(container._reactRootContainer); |
There was a problem hiding this comment.
Can you verify the logic here? This means if you call render on a container with existing React children created by another React, we don't warn? I guess that is consistent with before.
There was a problem hiding this comment.
Yeah we currently only warn about different copies of React on unmount
| it('should flush updates in the correct order across roots', () => { | ||
| var instances = []; | ||
| var updates = []; | ||
| var container = document.createElement('div'); |
There was a problem hiding this comment.
Hm. This looks wrong. This changes the behavior we're testing. Why doesn't it work?
| } | ||
|
|
||
| componentDidMount() { | ||
| this.container = document.createElement('div'); |
There was a problem hiding this comment.
Specifically we want to test that we can render into the node we already rendered.
There was a problem hiding this comment.
It does work the old way but it warns. Stack doesn't warn but it seems like it should?
Why doesn't Stack warn?
There was a problem hiding this comment.
It's allowed to render into an empty element. This test intentionally tests that so your change messes that up.
Only warn if the container has a React-rendered child.
acdlite
force-pushed
the
fibermounttests
branch
from
f09f632 to
9fcf761
Compare
|
@spicyj Fixed |
| ' This may be caused by unintentionally loading two independent ' + | ||
| 'copies of React.' : | ||
| '' | ||
| ) |
There was a problem hiding this comment.
Do these strings get compressed? Would it be better if we split them into separate invariants?
There was a problem hiding this comment.
It would be nice if error code transform was throwing on anything other than static messages.
There was a problem hiding this comment.
@gaearon: It does; this is a %s param so you can put whatever you want there.
There was a problem hiding this comment.
Yeah, I copy and pasted this from Stack. I'll change it in both places to use multiple invariants instead.
That way the messages are extracted by the error code transform.
5 participants
Fixes some of the tests in
ReactMount-test.js, except the ones related to server-side rendering.