Commit Graph

204 Commits

Author SHA1 Message Date
Andrew Clark
c6ccc19095 Use an UpdateQueue for top-level updates
...rather than mutate the pendingProps directly, which throws away any
previously scheduled work.
2016-12-19 16:33:16 -08:00
Andrew Clark
0b37588171 Revert "[Fiber] Queue top-level updates" 2016-12-19 15:32:59 -08:00
Andrew Clark
ddc44b7add Merge pull request #8584 from acdlite/fiberhostrootupdatequeue
[Fiber] Queue top-level updates
2016-12-19 13:56:18 -08:00
Brian Vaughn
1b868cbd71 Don't warn about class component usage of getInitialState if state is also set
We warn about getInitialState() usage in class components because users may have accidentally used it instead of state. If they have also specified state though then we should skip this warning.

Context: https://twitter.com/soprano/status/810003963286163456
2016-12-17 11:16:27 -08:00
Brian Vaughn
a5e728747e Merge pull request #8590 from bvaughn/unexpected-context-pop
`bailoutOnLowPriority` pushes context to mirror complete phase context pop
2016-12-17 07:41:25 -08:00
Dan Abramov
70f704dca6 [Fiber] Nesting validation warnings (#8586)
* (WIP) Nesting warnings

* Remove indirection

* Add a note about namespace

* Fix Flow and make host context required

This makes it easier to avoid accidental nulls.
I also added a test for the production case to avoid regressing because of __DEV__ branches.
2016-12-16 18:57:58 -08:00
Andrew Clark
a9365b97e5 Remove unmountContainer
Instead, renderers should pass null to updateContainer.
2016-12-16 18:31:39 -08:00
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
Brian Vaughn
441cc5c82b bailoutOnLowPriority correct pushes context for ClassComponents to mirror complete phase context pop 2016-12-16 15:04:06 -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