Commit Graph

196 Commits

Author SHA1 Message Date
Andrew Clark
d8ade83eb2 Use an UpdateQueue for top-level updates
...rather than mutate the pendingProps directly, which throws away any
previously scheduled work.
2016-12-16 16:30:28 -08:00
Andrew Clark
7b6c94c767 Don't drop updates from queue when calling replaceState
We should be able to support passing a function to replaceState, which
receives the accumulation of all the previously applied state updates.
Which means we shouldn't drop those updates from the queue. Technically,
we could drop them only when an object is passed to replaceState, but
that seems like more trouble than it's worth.
2016-12-16 16:20:16 -08:00
Andrew Clark
eec431297a Priority context during reconciliation
setState inside render/cWRP should have the same priority as whatever
level is currently being reconciled.
2016-12-15 09:12:13 -08:00
Andrew Clark
e0981b8bc5 Handle setState inside an updater function
The update is scheduled as if the current processing update has already
been processed; if it has the same or higher priority, it will be
flushed in the same batch.

We also print a warning.
2016-12-15 09:12:13 -08:00
Andrew Clark
20f00045d3 Apply pending updates in order of priority
The queue maintains a pointer to the last progressed update in the list.
Updates that come after that pointer are pending. The pointer is set to
the end of the list during reconciliation.

Pending updates are sorted by priority then insertion. Progressed
updates are sorted by the order in which they were applied during
reconciliation, which may not be by priority: if a component bails out
before the updates are committed, in the next render, the progressed
updates are applied in the same order that they were previously, even if
a higher priority update comes in.

Once a progressed update is flushed/committed, it's removed from
the queue.
2016-12-15 09:12:12 -08:00
Andrew Clark
94011e98a6 Separate priority for updates
When resetting the priority in the complete phase, check the priority of
the update queue so that updates aren't dropped.

Updates inside render, child cWRP, etc are no longer dropped.

The next step is sort the queue by priority and only flush updates that
match the current priority level.
2016-12-15 09:12:12 -08:00
Dan Abramov
e36b38c1ca [Fiber] Fix some of the warnings (#8570)
* Implement component stack for some warnings in Fiber

* Keep Fiber debug source up to date

When an element changes, we should copy the source and owner again.
Otherwise they can get stale since we're not reading them from the element.

* Remove outdated TODOs from tests

* Explicitly specify Fiber types to include in the stack

Fixes an accidental omission when both source and owner are null but displayName exists.

* Fix mised Stack+Fiber test to not expect extra warnings

When we're in Fiber mode we don't actually expect that warning being printed.

* Warn on passing different props to super()

* Implement duplicate key warnings

We keep known keys in a set in development. There is an annoying special case where we know we'll check it again because we break out of the loop early.

One test in the tree hook regresses to the failing state because it checks that the tree hook works without a Set available, but we started using Set in this code. It is not essential and we can clean this up later when we decide how to deal with polyfills.

* Move ReactTypeOfWork to src/shared

It needs to be available both to Fiber and Isomorphic because the tree hook lives in Isomorphic but pretty-prints Fiber stack.

* Add dev-only ReactDebugCurrentFiber for warnings

The goal is to use ReactCurrentOwner less and rely on ReactDebugCurrentFiber for warning owner name and stack.

* Make Stack invariant messages more consistent

Fiber used a helper so two tests had the same phrasing.
Stack also used a helper for most invariants but hardcoded a different phrase in one place.
I changed that invariant message to use a helper which made it consistent with what it prints in Fiber.

* Make CSSPropertyOperations use getCurrentFiberOwnerName()

This gets mount-time CSS warnings to be printed.

However update-time warnings are currently ignored because current fiber is not yet available during the commit phase.

We also regress on HostOperation hook tests but this doesn't matter because it's only used by ReactPerf and it doesn't work with Fiber yet anyway. We'll have to think more about it later.

* Set ReactDebugCurrentFiber during the commit phase

This makes it available during updates, fixing the last failing test in CSSPropertyOperations.

* Add DOM warnings by calling hooks directly

It is not clear if the old hook system is worth it in its generic incarnation. For now I am just hooking it up to the DOMFiber renderer directly.

* Add client-side counterparts for some warning tests

This helps us track which warnings are really failing in Fiber, and which ones depend on SSR.
2016-12-15 08:36:58 -08:00
Ben Alpert
ba8f24ba99 Prepare new composite child before removing old (#8572)
This matches what we do in Fiber -- and doing it this way is the only way we can prepare new views in the background before unmounting old ones.

In particular, this breaks this pattern:

```js
class Child1 extends React.Component {
  render() { ... }
  componentWillMount() {
    this.props.registerChild(this);
  }
  componentWillUnmount() {
    this.props.unregisterChild();
  }
}

class Child2 extends React.Component {
  render() { ... }
  componentWillMount() {
    this.props.registerChild(this);
  }
  componentWillUnmount() {
    this.props.unregisterChild();
  }
}

class Parent extends React.Component {
  render() {
    return (
      showChild1 ?
        <Child1
          registerChild={(child) => this.registered = child}
          unregisterChild={() => this.registered = null}
        /> :
        <Child2
          registerChild={(child) => this.registered = child}
          unregisterChild={() => this.registered = null}
        />
    );
  }
}
```

Previously, `this.registered` would always be set -- now, after a rerender, `this.registered` gets stuck at null because the old child's componentWillUnmount runs *after* the new child's componentWillMount.

A correct fix here is to use componentDidMount rather than componentWillMount. (In general, componentWillMount should not have side effects.) If Parent stored a list or set of registered children instead, there would also be no issue.
2016-12-14 11:14:50 -08:00
Gaëtan Renaudeau
931cad5aae Fix test renderer unmount (#8512)
* [react-test-renderer] unmount the inner instances

Fixes https://github.com/facebook/react/issues/8459

* add a test for https://github.com/facebook/react/issues/8459

* add new test in tests-passing.txt
2016-12-14 10:57:21 -05:00
Andrew Clark
97b7d0eff5 Test top-level callback when re-rendering with same element 2016-12-13 16:28:06 -08:00
Andrew Clark
a930f09dfe Give setState callbacks componentWillUpdate semantics
This matches the behavior in Fiber. Normally we would change Fiber
to match Stack to minimize breaking changes for the initial release.
However, in this case it would require too large a compromise to change
Fiber to act like Stack.
2016-12-13 15:50:33 -08:00
Sebastian Markbage
db489a4ade Only do runtime validation of tag names when createElement is not used
We introduced runtime validation of tag names because we used to generate
HTML that was supposed to be inserted into a HTML string which could've
been an XSS attack.

However, these days we use document.createElement in most cases. That
already does its internal validation in the browser which throws. We're now
double validating it. Stack still has a path where innerHTML is used and
we still need it there. However in Fiber we can remove it completely.
2016-12-13 14:53:36 -08:00
Sebastian Markbage
3092f63c6b Warn if upper-case tags are used
In #2756 we ended up using toLowerCase to allow case insensitive HTML tags.
However, this requires extra processing every time we access the tag or
at least we need to process it for mount and store it in an extra field
which wastes memory.

So instead, we can just enforce case sensitivity for HTML since this might
matter for the XML namespaces like SVG anyway.
2016-12-13 14:53:29 -08:00
Ben Alpert
ec8b94e847 Disable coverage on PRs on Circle (#8569) 2016-12-13 14:27:25 -08:00
Dan Abramov
ef532fd4a4 [Fiber] Fix portal bugs (#8532)
* Enable additional (failing) portal tests

* Fix portal unmounting

When unmount a portal, we need to unmount *its* children from itself.
This is similar to what we would do for a root if we allowed deleting roots.

* Skip portals when looking for host siblings

A portal is not part of that host tree despite being a child.

* Fix comment typo

* Add a failing test for portal child reconciliation

It is failing because portal bails out of update, seeing null in pendingProps.
It is null because we set pendingProps to nextPortal.children, which is null in this test.

* Fix the bug when switching to a null portal child

If pendingProps is null, we do a bailout in beginWork.
This prevents unmounting of the existing child when the new child is null.

We fix this by changing portal fiber's pendingProps to be the portal object itself instead of its children.
This way, it is never null, and thus doesn't cause a false positive in the bailout condition.

* Add a comment about HostPortal in getHostSibling

* Revert the fix because I don't know why it worked

unmountHostComponents() should have worked despite finding the wrong parent because it should have changed the parent when pushing and popping the portals.

* Don't call commitDeletion recursively

This leads to a "Cannot find host parent" bug because commitDeletion() clears the return field.
When we're inside the loop, we assign node.sibling.return to node.return but by this moment node.return has already been nulled.
As a solution we inline code from commitDeletion() without the nulling.

It still fails tests but for a different reason (unrelated bug).

* Skip portal children in commitNestedUnmounts()

We are currently already visiting them in commitUnmount() portal case since it's recursive.
This condition avoids visting them twice.

* Set node.child.return before going deeper

It doesn't seem to influence existing tests but we have this protection in all other similar places.
It protects against infinite loops.

* Revert "Fix the bug when switching to a null portal child"

This reverts commit ed9747deed.

I'll solve this by using an array in place of null instead.

* Use [] for empty Portal pendingProps

This avoids a false positive bailout with pendingProps == null when portal is empty.
2016-12-12 14:18:55 -08:00
Sebastian Markbåge
6c785ed524 Add unit tests for event bubbling in portals (#8509)
These bubble through the portal and up to the parent that rendered
the portal.
2016-12-08 13:51:21 -08:00
Sebastian Markbåge
3937bca8a2 Merge pull request #8491 from sebmarkbage/fibercurrenteventhandlers
[Fiber] Read Event Handlers from the "Current" Fiber
2016-12-08 13:49:51 -08:00
Dan Abramov
c87ffc0beb [Fiber] Support SVG (#8490)
* Test that SVG elements get created with the right namespace

* Pass root to the renderer methods

* Keep track of host instances and containers

* Keep instances instead of fibers on the stack

* Create text instances in begin phase

* Create instance before bailing on offscreen children

Otherwise, the parent gets skipped next time.
We could probably create it later but this seems simpler.

* Tweak magic numbers in incremental tests

I don't understand why they changed but probably related to us moving some work into begin phase?

* Only push newly created nodes on the parent stack

Previously I was pushing nodes on the parent stack regardless of whether they were already in current or not.
As a result, insertions during updates were duplicated, and nodes were added to existing parents before commit phase.
Luckily we have a test that caught that.

* Fix lint

* Fix Flow

I had to wrap HostContext API into a closure so that it's parameterizeable with I and C.

* Use the same destructuring style in scheduler as everywhere else

* Remove branches that don't seem to run anymore

I'm not 100% sure this is right but I can't get tests to fail.

* Be explicit about the difference between type and tag

I was confused by th HACK comment so I learned how DOM and SVG work with casing and tried to write a more descriptive comment.
It also seems like passing fiber.type into finalizeInitialChildren() is a potential problem because DOM code assumes tag is lowercase.
So I added a similar "hack" to finalizeInitialChildren() that is identical to the one we have prepareUpdate() so if we fix them later, we fix both.

* Save and restore host context when pushing and popping portals

* Revert parent context and adding children in the begin phase

We can address this later separately as it is a more hot path.
This doesn't affect correctness of SVG container behavior.

* Add a test for SVG updates

This tests the "jump" reuse code path in particular.

* Record tests

* Read ownerDocument from the root container instance

This way createInstance() depends on the innermost container only for reading the namespace.

* Track namespaces instead of creating instances early

While we might want to create instance in the begin phase, we shouldn't let DOM guide reconciler design.
Instead, we are adding a new concept of "host context". In case of ReactDOMFiber, it's just the current namespace.
We are keeping a stack of host context values, ignoring those that are referentially equal.
The renderer receives the parent context and type, and can return a new context.

* Pop child context before reading own context and clarify API

It wasn't quite clear from the API which context was being returned by the renderer. Changed the API to specifically ask for child context, and thus to pop it before getting the current context.

This fixes the case with <foreignObject> to which I intended to give SVG namespace.

* Give SVG namespace to <svg> itself

* Don't allocate unnecessarily when reconciling portals

We create stacks lazily so that if portal doesn't contain <svg>s, we don't need to allocate.
We also reuse the same object for portal host context state instead of creating a new one every time.

* Add more tests for edge cases

* Fix up math namespace

* Maintain a separate container stack

* Fix rebase mistakes

* Unwind context on errors

* Reset the container state when reusing the object

* Add getChildHostContext() to ReactART

* Record tests
2016-12-08 13:10:47 -08:00
Andrew Clark
f56eb89389 Add additional scheduling tests
Tests whether certain types of work can be performed after the deadline
has expired.
2016-12-07 23:09:08 -08:00
Andrew Clark
2508888a2b Errors thrown when detaching a ref should not interrupt unmount
If a ref throws while detaching, it should not prevent
componentWillUnmount from being called.

Additionally, errors thrown by removeChild should not be ignored, even
as the result of unmounting a failed subtree.
2016-12-07 23:09:08 -08:00
Andrew Clark
5fcdebf712 Move commit phase outside of performUnitOfWork
This gives us the ability to complete a tree without having to commit it
within the same frame.
2016-12-07 23:09:08 -08:00
Brian Vaughn
9510ecfc5e Merge pull request #8521 from bvaughn/react-art-fiber
New fiber-based ReactART renderer
2016-12-07 18:35:48 -08:00
Ben Alpert
4d71a9c580 Probably fix facts tracker 2016-12-07 16:04:47 -08:00
Brian Vaughn
81cb216288 Imported new ReactART fiber renderer
Split ReactART into stack and fiber targets. Added new ReactART test case for recently-fixed bug.
2016-12-07 15:35:53 -08:00
Brian Vaughn
9a3139121c Merge pull request #8520 from bvaughn/do-not-reset-inner-html-for-null-children
Do not reset inner html for null children
2016-12-07 15:35:13 -08:00
Brian Vaughn
bbdfc28002 Updated tests-passing 2016-12-07 15:34:32 -08:00
Sebastian Markbåge
343fb958f1 Add test for componentDidUpdate with a bailout in the middle (#8525)
This is one of the cases where Fiber diverges.
2016-12-07 15:14:32 -08:00
Ben Alpert
29a1707911 Support parallelism on Circle (#8511) 2016-12-07 14:55:34 -08:00
Ben Alpert
d8b591d584 Fix pull request detection on Circle 2016-12-07 13:45:06 -08:00
Brian Vaughn
2533fa8d35 Do not reset innerHTML for elements with null children
This is required to support certain third party scripts as well as other fiber renderers (eg the new ReactART renderer)
2016-12-07 12:06:06 -08:00
Ben Alpert
6ca66f70ca Update facts-tracker for Circle (#8510) 2016-12-06 22:24:19 -08:00
Sebastian Markbåge
facce3d736 Use Portals to test batching across roots (#8508)
This doesn't work by default in Fiber, you have to opt-in with a Portal to
get the explicit ordering guarantees.
2016-12-06 13:46:36 -08:00
Sebastian Markbage
ee4984980e Get the "current" Fiber from each node along the path
This fixes the issue where using the .return pointer isn't guaranteed to
return the current Fiber so we might read the wrong props when we try
to get the current event.
2016-12-02 20:03:09 -08:00
Sebastian Markbage
910044a41d Add failing event handler test
When we perform an update to the event handler we properly update the
immediate Fiber pointer of a child to be the current. However, when we
bubble events we use the return pointer which is not guaranteed to point
to the current Fiber even if we start from the current.

This manifests itself when we bailout in a parent. So I made the tests
use a PureComponent to illustrate this scenario. There is already a failing
case but I'm adding another one too.
2016-12-02 19:33:24 -08:00
Sebastian Markbåge
0723007e1e Merge pull request #8450 from sebmarkbage/fiberfinddomnode
[Fiber] Fix findDOMNode and findAllInRenderedTree
2016-12-02 16:34:46 -08:00
Ben Alpert
a104525e7f Only upload build if server exists (#8488) 2016-12-02 14:31:17 -08:00
Tom Occhino
153fe38403 Add circle.yml / CircleCI support (#8486) 2016-12-02 14:18:12 -08:00
Sebastian Markbage
a434f9e7af Fix findDOMNode and findAllInRenderedFiberTreeInternal
This strategy finds the current fiber. It traverses back up to the root if
the two trees are completely separate and determines which one is current.
If the two trees converge anywhere along the way, we assume that is the
current tree. We find the current child by searching the converged child
set.

This could fail if there's any way for both return pointers to point
backwards to the work in progress. I don't think there is but I could be
wrong.

This may also fail on coroutines where we have reparenting situations.
2016-12-01 21:21:05 -08:00
Sebastian Markbage
4fd1802ddf Additional findDOMNode tests
This is failing in Fiber without the fix. Because we have no deletions to rely
on in this case and the placement effects have already happened.
2016-12-01 21:03:24 -08:00
Ben Alpert
361ce5c00b Add test for unmount/remount in a single batch (#8470)
Fails in Fiber.
2016-12-01 14:19:00 -08:00
Ben Alpert
981f461b70 Merge pull request #8463 from spicyj/travis
Update Travis config for new token
2016-11-30 21:03:01 -08:00
Ben Alpert
cb2927665c Fix insecure shell escaping in facts tracker 2016-11-30 21:02:44 -08:00
Andrew Clark
aa6279c41a Clear existing text content before inserting children (#8331)
Fixes a bug when updating from a single text child (or
dangerouslySetInnerHTML) to regular children, where the previous
text content never gets deleted.
2016-11-30 18:08:41 -08:00
Ben Alpert
06e8cedc0d Update Travis config for new token 2016-11-30 17:17:04 -08:00
Andrew Clark
e7eed5874f [Fiber] New error boundary semantics (#8304)
* Exit early in scheduleUpdate if a node's priority matches

This is a performance optimization and is unobservable. However, it
helps protect against regressions on the following invariants on which
it relies:

- The priority of a fiber is greater than or equal to the priority of
all its descendent fibers.
- If a tree has pending work priority, its root is scheduled.

* New error boundary semantics

- Recovering from an error boundary no longer uses Task priority by
default. The work is scheduled using whatever priority created the
error.
- Error handling is now a side-effect that happens during the
commit phase.
- The default behavior of an error boundary is to render null. Errors
do not propagate except when an boundary fails. Conceptually, this would
be like throwing an error from a catch block.
- A host container is treated like a no-op error boundary. The first
error captured by a host container is thrown at the end of the batch.
Like with normal error boundaries, the entire tree is unmounted.

* Fix broken setState callback test

* Add test for "unwinding" context when an error interrupts rendering

* Switch over primary effect types only

This avoids the need to create an export for every combination of bits.

* Only continue the work loop if the error was successfully captured

* Add more tests for incremental error handling

These tests are currently failing:

  ✕ catches render error in a boundary during partial deferred mounting
  ✕ catches render error in a boundary during animation mounting
  ✕ propagates an error from a noop error boundary during full deferred mounting
  ✕ propagates an error from a noop error boundary during partial deferred mounting
  ✕ propagates an error from a noop error boundary during animation mounting

The observed behavior is that unstable_handleError() unexpected gets called twice:

      "ErrorBoundary render success",
      "BrokenRender",
      "ErrorBoundary unstable_handleError",
  +   "ErrorBoundary render success",
  +   "BrokenRender",
  +   "ErrorBoundary unstable_handleError",
      "ErrorBoundary render error"

* Verify batched updates get scheduled despite errors

* Add try/catch/finally blocks around commit phase passes

We'll consolidate all these blocks in a future PR that refactors the
commit phase to be separate from the perform work loop.

* NoopBoundary -> RethrowBoundary

* Only throw uncaught error once there is no more work to perform

* Remove outdated comment

It was fixed in #8451.

* Record tests

* Always reset nextUnitOfWork on error

This is important so that the test at the end of performAndHandleErrors() knows it's safe to rethrow.

* Add a passing test for unmounting behavior on crashed tree

* Top-level errors

An error thrown from a host container should be "captured" by the host
container itself

* Remove outdated comment

* Separate Rethrow and Noop scenarios in boundary tests

* Move try block outside the commit loops
2016-11-30 16:54:20 -08:00
Ben Alpert
705c9bcfd2 Refs between fiber and stack (#8458)
I feel gross. Better ideas welcome.
2016-11-30 12:17:03 -08:00
Dan Abramov
b1c988d0de Add tests for recovery from errors thrown in the reconciler (#8462)
Test that errors in the reconciler can be caught by error boundaries, and that we can still schedule updates if they are uncaught.
2016-11-30 17:00:14 +00:00
Dan Abramov
623f608aab [Fiber] Make bad element type message same as in Stack (#8460)
* Make bad element type message same as in Stack

This makes Fiber emit the same message as Stack (aside from the missing owner information).

* Add a separate test verifying error includes owner name

Fiber currently doesn't pass it. This is just to keep track of it as a todo.
2016-11-30 16:29:42 +00:00
Ben Alpert
6110c58584 Fix reordering of bailed out children (#8457) 2016-11-29 14:57:04 -08:00
Ben Alpert
16bf429029 Fix fiber feature flag for ReactDOMProduction-test (#8451) 2016-11-28 23:36:05 -08:00