* Limit the number of nested synchronous updates
In Stack, an infinite update loop would result in a stack overflow. This
gives the same behavior to Fiber.
Conceptually, I think this check belongs in findNextUnitOfWork, since
that is what we call right before starting a new stack. I've put it
in scheduleUpdate for now so I have access to the component that
triggered the nested update. But we could track that explicitly instead.
I've chosen 1000 as the limit rather arbitrarily. Most legit use cases
should really have a much smaller limit, but a smaller limit could break
existing code. For now I'm only concerned with preventing an infinite
loop. We could add a warning that fires at the smaller limit.
* Move check to findNextUnitOfWork
Including the name of the component in the message probably isn't
necessary. The JS stack will include either componentDidUpdate or
componentWillUpdate. And the component that's updating won't
necessarily be the component whose lifecycle triggered it.
So let's move the infinite loop check to findNextUnitWork as I
originally wanted to.
* 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 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.
* 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.
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.
This is a follow-up on
https://github.com/facebook/react/pull/9584#discussion_r115642293. There
is no need to assign the value property of an input if the value
property of the React component changes types, but stringifies to the
same value. For example:
```javascript
DOM.render(<input value="true" />, el)
DOM.render(<input value={true} />, el)
```
In this case, the assignment to `input.value` will always be
cast to the string "true". There is no need to perform this
assignment. Particularly when we already cast the value to a string
later:
```javascript
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;
```
* Don't double validate the DOM container
The warnings need to check a valid container but that should happen in
unstable_renderSubtreeIntoContainer too so might as well move it in.
* Hydrating DOM
Hydrates a server-rendered DOM tree by traversing it and connecting up the nodes if they match. Attributes are not diffed. They're assumed to be accurate on matching nodes.
* Remove meta data filtering from test
Because the current server renderer includes the meta data, it remains in a revived tree.
* Annotate because Flow
* Don't track insertion effects if we're going to hydrate
During initial mount, we should not track Placement
* Fix up test cases to ignore errors that we no longer throw
TODO make these warnings instead.
* Correctly track hydration state inside a newly inserted tree
When we don't match the first hydration node, we'll do an insertion.
Currently we keep the next hydratable sibling so that we know where to
pick up once we're done with the insertion. Unfortunately this makes the
nodes inside the insertion think that there's a node to hydrate.
I used to check for the direct parent but that doesn't work on nested host components.
We need to instead keep track of that we're in an hydration context but
we're not currently hydrating. Once we pop passed the inserted node can
we resume hydrating.
* Hacky fix to isMounted
isMounted checks whether a component is inside a Placement. During
hydration we ideally don't do any Placements if hydration matches the tree.
To work around this I use the Placement flag on the root, which isn't used
for anything else. But only temporarily while we're hydrating. Then reset
it before committing.
* Record tests
* Comments
* Adjust some expectations of the server markup format of Fiber
Currently this case is using the stack renderer.
* Ensure debug hooks are injected into the Stack server renderer
In our tests this normally happens because ReactDOM.js injects them into
the shared module, but when Fiber is enabled or this is its own flat
bundle, that doesn't happen.
* Add package builds for new server renderer and enable tests
ReactServer -> ReactDOMServerStream
This file is going to be the replacement for ReactDOMServer.
I mock ReactDOMServer and user ReactDOMServerStream when we have
the fiber flag enabled. I'm now also enabling this as the default for
distributions builds (react-dom/server on npm and
react-dom-server.production.min.js as umd bundle).
I'm using traverseStackChildren instead of traverseAllChildren because
traverseAllChildren is now only in the isomorphic package and we don't
want to build all of that that into the server package.
I also have to require lower case react for the builds to work.
`ReactEventListener` is a DOM-specific module although the name suggests
otherwise. This change renames the module to the more specific
`ReactDOMEventListener`.
Recreating the class instance causes refs (and other callbacks) to close
over stale instances.
Instead, re-use the previous instance. componentWillMount is called
again. We also call componentWillReceiveProps, to ensure that
state derived from props remains in sync.
* Remove loose check when assigning non-number inputs
This commit removes a check I added when working on number input
issues where we perform a loose check on an input's value before we
assign it. This prevented controlled text inputs from disallowing
numeric text entry.
I also added a DOM fixture text case.
Related issues:
https://github.com/facebook/react/issues/9561#issuecomment-298394312
* Use strict equality as a guard before assigning input.value
This commit adds back the guard around assigning the value property to
an input, however it does it using a strict equals. This prevents
validated inputs, like emails and urls from losing the cursor
position.
It also adds associated test fixtures.
* Add copy command after build for interup with surge.sh
Latest versions of prop-types don't depend on React, so the factory is
not necessary, and in fact bloats the build because it is intended for
15.5 and so doesn't strip out the checkers in prod.
- Update examples to no longer use React.DOM
- Add package and documentation entries for react-addons-dom-factories
- Update dom-factories readme
- Set up proxy to intercept React.DOM usage
- Update ReactDOM children tests to use createElement
- Add more specific warning assertion for React DOM factories
- Do not use expectDev in ReactDOMFactories tests
* Add test for React.PureComponent
* Add warning when shouldComponentUpdate is declared in a PureComponent
* Add actionable warning
* Add warning in Fiber
* Format added code by running yarn prettier
* Move pure sCU check to checkClassInstance
That way it warns before the component updates
Andrew Clark
Flarnie Marchan
Garrett McCullough
Dheeraj Kumar
Eli White
Andrew Clark
Sebastian Markbåge
Nathan Hunzaker
Ben Alpert
Sasha Aickin
Bogdan Chadkin
Dan Abramov
Dominic Gannaway
pingan1927
Edvin Erikson
Brian Vaughn
Dustan Kasten
Tom Occhino
Toru Kobayashi
Philipp Spieß
Manas
Soo Jae Hwang