Fixed all eslint warnings #7230
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Warnings included - Missing semicoln warnings - Line exceeding 120 characters Signed-off-by: Usman Ajmal <syedusmanajmal@gmail.com>
ghost
added
| @@ -55,7 +55,10 @@ var DOMRenderer = ReactFiberReconciler({ | |||
| return domElement; | |||
| }, | |||
|
|
|||
| prepareUpdate(domElement : Instance, oldProps : Props, newProps : Props, children : HostChildren<Instance>) : boolean { | |||
| prepareUpdate(domElement : Instance, | |||
There was a problem hiding this comment.
Please put each argument on its own line indented 2 space, closing paren with bracket on last line unindented
fn(
arg,
arg
) {There was a problem hiding this comment.
That's a better style. I will push a patch. :)
|
I like the idea and we'll take this if you update to match our style. Thanks :) |
|
Couldn't figure out a way to push patch here to this issue so created a new pull request: #7247. Hope that's acceptable. :) |
|
You should be able to just add more commits and push to the same branch. If you can fix that I'd prefer it since your new PR is also going to need more changes and creating a new PR for every followup isn't maintainable. |
|
@zpao Yes, sorry about that. I have closed the new non-necessary PR and will update this one in a while. |
Because pattern matching or something.
This flag on fibers will be used to track what priority of work is needed by that subtree, if any at all. Also fix up the TypeOfWork to have consistent naming and typing.
I tried to be clever and generalize it but this is currently only props and there are other assumptions that might break down if it isn't.
This is essentially equivalent to the current top level wrappers. They contain the next children to be mounted into a container node from the host. It is the responsibility of the host to retain references to them for updates. I expect them to be able to exist in the middle of the tree in the future, for renderSubtreeIntoContainer.
This automatically downgrades the priority of a hidden node. Its children won't be reconciled until they come around the next time.
This is a bit poorly structured. I'll restructure when the pieces are better in place. Basically we reset the priority of a node before work on the children. The children then bump their parent if they end up having work left. This is the first time we're seeing deep updates happening. The new unit test demonstrates this. There is an interesting case that happens when we fall back out of a deep update. We end up "completing" a node that we didn't begin. This probably breaks in coroutines. When that completes, it'll try to render the sibling next but that should bail out so we check for any pending work on the sibling. That one I'm not sure about.
We need a canonical stateful root for each. I don't really want to overload the HostContainer for this purpose since it makes the fiber code more specialized. Instead I create a root which represents an actual stateful root. When these get scheduled they get chained together in a linked list. However, we don't hold onto anything that doesn't have scheduled work. This will help us release everything automatically in the GC, as long as there are no subscriptions nor scheduled work.
The scheduler is getting quite complicated. I'll extract it into its own module.
This fixes some bugs with the clones and traversing backwards through them. It is important that we maintain the correct parent at all times and that clones have the correct values. We need to carefully clone everything on the way up to the the fiber with the next work to do. This code is a bit messy and fragile now. I'm sure I didn't get it all right but I want to get the basics in place first. Then we can structure this part better. I think the general algorithm is sound though.
It is helpful to be able to dump information about the current tree for debugging issues in unit tests.
First I fix a bug where host components didn't properly bail out although this was unobservable. When we bail out, we need to ensure that we preserve the highest remaining priority work that is left to do for that subtree. This still isn't properly handling the case when that work has the *same* priority as the current one. That work will be flushed the *next* tick instead of the current pass. I can't create a test for that yet since I need setState to get to that state.
I found a way to test this case without any need for setState.
The parent pointer is updated to one of the two versions during work so if you log in the middle of work, it gets confused.
This tries to reuse work that was completed but another higher priority event came in. This tries to avoid starvation when high priority events causes low pri work to keep rerendering from scratch.
This is not just the parent Instance but also the return Fiber for some piece of work. This clarifies and buys into this definition. Basically, in the current model you will always pass into a fiber from the parent that you're going to return to. Even if you get aborted and reused this will be updated to the correct return fiber before you get back here. I don't have any guarantees in place to enforce this right now. I don't really know how to, but seems safe. :) I confirmed that the use of keyword properties work for old engines because we have the transform enabled in our build system.
The src/core folder moved while this PR was pending so this file didn't move with it. Let's get rid of this annoying top level folder.
Rename secret!
* Add failing tests for facebook#7187 and facebook#7190 * Pass shouldHaveDebugID flag to instantiateComponent This allows us to remove a hack that was added in facebook#6770 and caused facebook#7187 and facebook#7190. * Move logic for choosing whether to use debugID outside instantiate
This provides an easy way to indicate that components should only rerender when given new props, like PureRenderMixin. If you rely on mutation in your React components, you can continue to use `React.Component`. Inheriting from `React.PureComponent` indicates to React that your component doesn't need to rerender when the props are unchanged. We'll compare the old and new props before each render and short-circuit if they're unchanged. It's like an automatic shouldComponentUpdate.
* Add failing test demonstrating a ReactPerf warning * Make the failing ReactPerf test more precise * Make ReactPerf.start() work during reconciliation * Reorder lifecycle methods for greater clarity * Fix memory leak * Error boundaries should not break ReactPerf * Put onBeginFlush/onEndFlush into transaction wrappers This looks cleaner even though it is not strictly necessary. We still call them manually for unmounting because it doesn't have a transaction.
Warnings included - Missing semicoln warnings - Line exceeding 120 characters Patch 2: - Fixed styling of lines exceeding 120 chars limit Signed-off-by: Usman Ajmal <syedusmanajmal@gmail.com>
Warnings fixed: - Missing semicoln warnings - Line exceeding 120 characters Followed FB style for avoid longer lines. grunt lint: PASS grunt test: PASS Signed-off-by: Usman Ajmal <syedusmanajmal@gmail.com>
…t-fixes Signed-off-by: Usman Ajmal <syedusmanajmal@gmail.com>
|
Not sure if I did the correct thing here but I had merge my changes with remote so I did following:
Please advise if this isn't acceptable. I will modify the commit. |
|
Thanks! Since we can squash & merge this should be ok. Otherwise you would need to rebase first to remove all of the extraneous commits. |
| returnFiber : Fiber, | ||
| existingChild : ?Fiber, | ||
| previousSibling : Fiber, | ||
| newChildren, priority : PriorityLevel |
There was a problem hiding this comment.
Can you move priority to a new line
|
Thanks! |
zpao
pushed a commit
that referenced
this pull request
Jul 13, 2016
(cherry picked from commit c52a2b9)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Warnings included
Signed-off-by: Usman Ajmal syedusmanajmal@gmail.com