* WIP Improve error message thrown in Fiber with multiple copies of React
**what is the change?:**
Adding an 'invariant' with detailed error message for the problem that
occurs when you load two copies of React with the Fiber reconciler.
WIP:
- Is there any other likely cause for this error besides two copies of
React?
- How can we make the message more clear?
Still TODO:
- Write a unit test
- Write a documentation page and create the link to the page, similar
to https://facebook.github.io/react/warnings/refs-must-have-owner.html
It would also be nice to have a page with instructions on how to fix
common cases of accidental double-loading of React, but we can open a
separate issue on that and let the community do it.
**why make this change?:**
This error comes up relatively often and we want to make things clear
when it happens in v16.0+
**test plan:**
WIP
**issue:**
Fixes#9962
* Add improved warning and docs for 'refs must have owner' in Fiber
**what is the change?:**
- Added warning in the place where this error is thrown in Fiber, to
get parity with older versions of React.
- Updated docs to mention new error message as well as old one.
I started to write a new docs page for the new error, and realized the
content was the same as the old one. So then I just updated the existing
error page.
**why make this change?:**
We want to avoid confusion when this error is thrown from React v16.
**test plan:**
- manually inspected docs page
- manually tested in a CRA to trigger the error message
(Flarnie will insert screenshots)
**issue:**
Fixes#9962
Related to #8854
* Add test for the informative warning around multiple react copies
@gaearon debugged the test for this and now it works!!!!!!!!!!!!!!!
!!!!!!!!!!!!!!!!!!!!!!!!!!!!! :)
**what is the change?:**
We now test for the warning that the previous commits add in Fiber, and
also test for the old warning in the stack reconciler.
**why make this change?:**
This is an especially important warning, and we want to know that it
will fire correctly.
**test plan:**
`yarn test src/renderers/__tests__/multiple-copies-of-react-test.js`
`REACT_DOM_JEST_USE_FIBER=1 yarn test src/renderers/__tests__/multiple-copies-of-react-test.js`
* Fix up test for 'multiple copies of react'
**what is the change?:**
refactor test for 'multiple copies of React' to be simpler and remove
some copypasta
* run prettier
* Fix conditionals in 'multiple copies of react' test
**what is the change?:**
When moving the 'fiber' and 'non-fiber' conditions from two assertions
into one, we copy pasted the wrong message into the 'fiber' condition.
This wasn't caught because we were using an outdated name for the
'fiber' constant when running the tests locally with fiber enabled.
This fixes the copy-paste error and we now are actually running the
tests with fiber enabled locally.
* Run scripts/fiber/record-tests
* change the argument passed to CallbackQueue.getPooled
* remove undefined from function call
* add test for ReactNativeReconcileTransaction
* update log of tests
* change test to one that operates on setState
* added new tests and fixed another instance of the bug
* run prettier
* update names of tests and minor clean up
* remove arg from CallbackQueue and update tests
* Add react-dom-unstable-native-dependencies
react-native-web and react-primitives currently access a few internals
for shimming DOM events into native ones. Changes in react@16 packaging
hide these internals completely. This change adds a submodule to react-dom,
unstable-native-dependencies that includes the necessary modules to
continue enabling that method of dom-native event injection.
* Update ResponderEventPlugin to use "public" interfaces for test
In order to get some sort of smoke testing on
react-dom-unstable-native-dependencies, update ResponderEventPlugin-test
to use the "public" interfaces provided by react-dom and the new
react-dom/unstable-native dependencies
Also adds the missing references in package.json as well as missing
files required for unittests to do imports correctrly
Also exports injectComponentTree() which is required for the unittests
to re-set the shared component state between runs.
* Tweak bundle comment
* Bundle content updates from exporting injectComponentTree
* Added FB_DEV, FB_PROD to bundle types
* Run yarn prettier for -unstable-native-dependencies updates
* Add failing test to show that shallow test renderer doesn't call setState's callback arg
* Record tests
* Fix shallow renderer's setState/replaceState/forceUpdate to execute any callbacks passed. (#10089)
* Ensure shallow renderer callbacks are called with the correct binding.
To make sure we don't reset the priority of a down-prioritized fiber,
we compare the priority we're currently rendering at to the fiber's
work priority. If the work priority is lower, then we know not to reset
the work priority.
* Warn for text content
* Warn for different attributes/properties values
Warns if there are unknown extra attributes in the hydrated node.
It also tries to compare the existing property or attribute against the
expected value. It does this by reading the property and comparing it to
the prop. Except it's not that simple because multiple prop values can
yield the same output. For this we pass an extra expected value that is
a hint as to which one was used. This is a bit weird but I'm not sure the
alternatives were much better.
* Warn when there is an insertion or deletion during hydration
This warns if there is ever an insertion or deletion due to hydration
failing to find a match.
Currently we can't warn for insertions required into the root because
that's how we do all non-hydrating renders atm. Left a todo.
This strategy is a bit unfortunate that it leads to so much plumbing code.
And we have to add three extra methods to the HostConfig that are only used
in DEV and not for anything else. I don't really have a better idea.
* Don't try to delete children of a textarea
Textareas are special cases. The initial mount inserts children
as the default value, but we should leave that untouched. This is the same
as the special case where we set text content of children so I'll use that
mechanism.
* Change expected format for text differences
In Stack this is presented as HTML which needs to have normalized escaping
rules. In Fiber it is currently not presented as HTML but a raw string
so we don't escape it.
* Unmount component in between tests
In Fiber, the second warning isn't issued because it's considered an update
not a new initial render and we don't fire the warning for those.
* Change expectation of white space text behavior in Fiber
In Fiber we don't expect empty strings to be different from rendering null.
In fact, in a follow up I plan on formalizing this by never creating text
Fibers for empty strings.
* Warn for different dangerouslySetInnerHTML
We can't just compare the raw innerHTML value because it will have been
normalized. Instead, we'll create another element, set its innerHTML and
read it back.
Since there can be custom elements registered with this document, we want
to avoid any side-effects they might cause. So I do this in a fresh new
document.
I'm not sure how this would affect controlled components and other stuff
that could have changed after runtime. I think for those cases maybe we
just need a general way of opting out of the diff.
* Warn if unmounting a non-container
* Warn if the 2nd+ child is a "root" element but not first
This triggers our non-reuse mode. This is covered by ReactMount already but
the test doesn't pass yet without also landing #10026.
This covers up errors that are thrown in Fiber, because callback gets
fired *and* an error is thrown. Created a follow up #10049 to reevaluate
these semantics.
# Conflicts:
# scripts/fiber/tests-passing-except-dev.txt
# scripts/fiber/tests-passing.txt
There's no advantage to scheduling updates with animation priority
versus scheduling sync work inside requestAnimationCallback. So we can
remove all the animation-specific code. Now there's only one type of
callback.
Allows us to make assertions on the values that are yielded when
performing work. In our existing tests, we do this manually by pushing
into an array.
ReactNoop.flushThrough extends this concept. It accepts an array of
expected values and flushes until those values are yielded.
* Prevents adding units to css custom properties
* Fix code style
* Optimize custom property checking
* Prevents adding units to css custom properties in markup creation
* Update passing tests
* Fix argument name and reuse check in DEV
* Remove internal forwarding modules for /lib/
* Add *Entry suffix to all entry points
* Don't bundle ReactNativeFeatureFlags since it's shimmed
* Delete TestRendererStack
* Switch tests at forwarding modules rather than via Jest
* Share mocks between regular and equivalence fixtures
* Rename environment flag to be more generic
* Remove accidental variable name change
* Minor naming changes for consistency
Files that have two versions get the engine in variable name.
* Make ReactControlledValuePropTypes DEV-only
* Remove canDefineProperty
This breaks IE8. We don't support it.
* Remove getNextDebugID
It was added temporarily to avoid Stack shared state issues across renderers.
Not a problem anymore.
* Make KeyEscapeUtils.unescape() DEV-only
* Remove unused deprecated() module
It's unlikely we'll deprecate anything else on React.* object soon.
* Inline getIteratorFn at the call sites
* Inline ReactElementSymbol
* Inline KeyEscapeUtils into Children and move the file into Stack
It's only used in one place in isomorphic.
It's used more broadly in Stack so we move it there to die.
* Update artifacts
* Reorder declarations for consistency
* Fix Flow
* Add check for string and null 'rootElement' in ReactDOMFiber
**what is the change?:**
Before we call 'rootElement.getAttribute' we check that the method is
defined.
**why make this change?:**
There is an internal use case I found where 'rootElement' is a string
and null at different points as the page is rendered.
It looks like this method was added as part of support for re-hydration
of server-side rendered content. I can't imagine we would want to reuse
content if the rootnode is a string or null. Not sure if we want an
earlier check that it's an element before this point.
**test plan:**
`yarn test`
and I manually tested this fix in the internal case where it was
breaking
* Add test and improve check for non-element rootElement
**what is the change?:**
We use the nodeType to check that we have the correct type of
rootElement, and we added a unit test.
**why make this change?:**
Improve this solution to the problem.
**test plan:**
`yarn test`
* run ./scripts/fiber/record-tests
* Don't hydrate any properties other than event listeners and text content
This strategy assumes that the rendered HTML is correct if the tree lines
up. Therefore we don't diff any attributes of the rendered HTML.
However, as a precaution I ensure that textContent *is* updated. This
ensures that if something goes wrong with keys lining up etc. at least
there is some feedback that the event handlers might not line up. With
what you expect. This might not be what you want e.g. for date formatting
where it can different between server and client.
It is expected that content will line up. To ensure that I will in a follow
up ensure that the warning is issued if it doesn't line up so that in
development this can be addressed.
The text content updates are now moved to the commit phase so if the tree
is asynchronously hydrated it doesn't start partially swapping out. I use
the regular update side-effect with payload if the content doesn't match up.
Since we no longer guarantee that attributes is correct I changed the
bad mark up SSR integration tests to only assert on the textContent
instead.
* Hydrate text node if possible
Currently we're never matching text nodes so we need to properly branch.
* Tweak syntax in rollup build script
@bvaughn and I already discussed this.
**test plan:**
`yarn build`
* Remove JSDoc comments
**what is the change?:**
removing some comments
**why make this change?:**
The code is basically self explanatory and these comments could get out
of sync.
**test plan:**
Visual inspection, `yarn build` and `yarn test`
**issue:**
https://github.com/facebook/react/issues/9398
According to #9836 we're intentionally chosing to not support this until
we have better proof of this being a big need. E.g. to protect against
extensions. In a way that it's not better to push extensions to be fixed.
This was mostly used for timing of initial mounts. However, we haven't
implemented that in Fiber and yet nobody has complained despite running
without it. Further more the update tracks any update within the tree,
not just updates to the props of the top level. This is much less useful
due to the variation.
I could make this track initial mounts too but it's a bit awkward so I'd
rather just delete it if possible. We can run the full profiling mode if
we want more coverage.
