React: [Fiber] Umbrella for remaining features / bugs

Created on 10 Oct 2016 · 10Comments · Source: facebook/react

This is an umbrella issues for remaining fiber issues. More can be found by running the unit tests with the useFiber flag in ReactDOMFeatureFlags turned on

See #8830 for additional tasks beyond the scope of the initial Fiber release

Phases 1–6

Phase 1: Infrastructure

[x] Set up infra to know which tests are failing @spicyj
[x] ReactTestUtils features
- [x] reactComponentExpect @gaearon #8258
- [x] scry etc. @gaearon #8257

Phase 2: Smaller / Initial Tasks

[x] ReactDOMFiber.render - Simple synchronous case. [@sebmarkbage] #8029
[x] String refs using owner. [@acdlite] #8099
[x] Pass the correct previous state to componentDidUpdate. #7941
[x] componentWillMount, componentWillReceiveProps, componentWillUpdate life-cycles. [@sebmarkbage] #8015
[x] componentDidUpdate should not fired if shouldComponentUpdate returns false. [@sebmarkbage] #8028
[x] Instance should be recreated when already started work gets resumed (no componentWillReceiveProps without didMount). [@sebmarkbage] #8015
[x] Fix module pattern in mountIndeterminateComponent. [@sebmarkbage] #8015
[x] findDOMNode. #8083 [@sebmarkbage]
[x] Switching between types doesn't track deletions (updateSlot doesn't return null). [@sebmarkbage / @gaearon] #8085
[x] PureComponent flag. [@acdlite] #8118
[x] Ensure that scheduling works synchronously by default but batched in DOM events. (Initial default priority.) [@acdlite] #8127
[x] unstable_batchedUpdates [@acdlite] #8127
[x] setState in componentDidMount/componentDidUpdate should always be synchronous at the end of the batch. Even if work is deferred. [@acdlite] #8206
- [x] Make sure setState + error boundaries work in it [@gaearon / @acdlite] #8193
[x] Unmount failed trees before attempting to recover [@acdlite] #8304

Phase 3: Larger Tasks

[x] Allow testing Fiber in Facebook web codebase [@spicyj]
[x] Error boundaries. [@gaearon]
[x] Context [@gaearon #8272]
[x] DOM attributes/properties update. [@sebmarkbage]
[x] DOM events - Plug in the event system. [@sebmarkbage]
[x] renderContainerIntoSubtree - Preserve state and such. [@spicyj #8368]
[x] [fb] Bug: Switching from text children to regular children won't clear the old text content. (Not an easy fix since the side-effect of the parent is scheduled after the child insertions.) [@acdlite / @sebmarkbage] #8331
[x] Recover from errors during commit phase, including errors thrown from host environment. Should only use a single try-catch block. [@acdlite]
[x] [fb] SVG mode switch (based on if a parent is SVG or HTML). [@gaearon #8417]
[x] [fb] setState behavior when called in all possible life-cycles including sibling life-cycles and parents and children. [@acdlite]

Phase 4: Uncovered Bugs

[x] Root container is wrong in commit phase (https://github.com/facebook/react/pull/8568#issuecomment-266848256) [@sebmarkbage] #8607
[x] Update state/props object pointers even if bailout happens. This will currently cause an unnecessary componentDidUpdate. Solve that somehow. [@acdlite] #8655
[x] ~~Validate if inputValueTracking does indeed need to be cleaned up on unmount~~ I don't think it needs to but we should revert anyway
[x] autoFocus doesn't work. Todo in ReactDOMFiberComponent. [@bvaughn] #8646
[x] [fb] When the last effect renders, there's an infinite loop. [@acdlite]
[x] [fb] Defer clearing of initial nodes until work is actually begun. (Ensure that unmounting and rerendering in the same batch works.) [@acdlite]
[x] Updates on different roots in componentWillMount (ReactUpdates-test) [@acdlite]
[x] Refs should update even if shouldComponentUpdate returned false. However, componentDidUpdate should not fire. [@acdlite] #8685
[x] [fb] Make top level render and unmount synchronous by default even when they're in a batch. (Even for updates? No, not for updates.) [@acdlite] #8634
[x] [fb] Uncaught TypeError: Cannot read property 'children' of null https://github.com/facebook/react/blob/705c9bcfd2745749a603b72e04528095021a673b/src/renderers/shared/fiber/ReactFiberBeginWork.js#L213 (probably due to context change, based on the caller) We should probably either avoid rerendering or set pendingProps to memoizedProps in this case. [@sebmarkbage] #8613
[x] Does replaceState remove any previous callbacks scheduled? [@acdlite]
[x] Root fiber should use updateQueue instead of pendingProps. [@acdlite]
[x] [fb] Ensure that we get the current props for controlled components in restoreStateIfNeeded. [@spicyj] #8443
[x] [fb] Iterable children. (E.g. immutable.js / Set etc.) [@gaearon #8446]
[x] [fb] Reuse HostText when bailing out [@sebmarkbage] #8371
[x] [fb] Ensure that renderSubtreeIntoContainer twice enables the context to pass through the middle subtree. I.e. two nested layer with a single context provider at the very top. [@spicyj] #8407
[x] Make HostContainer append/insert/remove child instead of always updating all. Detach .return when a child gets unmounted (removed). [@sebmarkbage] #8400
[x] Handle selection restoration #8401 #8353
[x] Swap out the active Fiber when a host node is updated - including if listeners change so that we can know which listener is active. _This needs to remain even when prepareUpdate is fixed._
[x] Remove traversal of .output. #8406
[x] findDOMNode and findAllInRenderedTree are broken. If an insertion happens and we happen to look at the previous work in progress, then it will not have any deletes or removes. So it looks like it is current. I assume the same can happen for a deletion after it is done. [@sebmarkbage] #8450
[x] ~~[fb] Batch different roots together into one commit (ReactUpdates-test)~~ Won't fix. Use Portals instead. #8508
[x] Error handling when an error is thrown in detachRef callbacks. [@acdlite]
[x] Ensure that if we walk up .return in TreeTraversal we always get the current Fiber for props, or possibly just walk up the parentNode tree instead. [@sebmarkbage] #8491
[x] ~~Ensure that when we switch on tag names, we cover mixed/upper case. E.g. React.createElement('INPUT').~~ Won't fix. Instead, we'll warn if upper case is used on HTML tags. [@sebmarkbage] #8563
[x] Some scheduling issue is causing ReactCompositeComponentNestedState-test to fail. It performs the first setState before the second one. Will any of the Task priority stuff fix this? (@acdlite)
[x] [fb] Scheduling an update during render doesn't work. We mostly already warn if setState is called from render, but it might required that componentWillMount (and others) calling a callback which sets state on a different component works. Currently the complete phase resets both the updateQueue and pending priority. This can cause infinite loops to happen. Apparently still used. We need to either warn/log better and upgrade everyone or support it in Fiber. [@acdlite]
[x] Top-level context push/pop isn't properly matched up; lots of tests fail if you patch in this. #8568 [@bvaughn]
[x] [fb] Reentrant mounting in synchronous mode [@spicyj #8623]

Phase 5

[x] Feature flag to disable Fiber-only features [@acdlite] #8797
[x] Polyfills or alternate paths for ~~Map, requestAnimationFrame~~ and requestIdleCallback. [@sebmarkbage] #8833
[x] isMounted - technically not deprecated yet. #8083 [@sebmarkbage]
[x] Top level render callbacks. Second argument to render. #8102 [@koba04]
[x] Ensure that server rendering works using Stack when Fiber is enabled. [#8372]
[x] Declarative portal API @gaearon #8386
- [x] Add tests for event bubbling (This will need to track #8117 for changes to how top level event listeners get attached.)
[x] React Test Renderer support. (@iamdustan #8628)
[x] React ART support. #8521 (@bvaughn)
[x] Land basic React Native support in React repo. #8560 (@bvaughn)
[x] Mark root renders during mount (perf testing) (see tests for "marks top-level updates") [@bvaughn] #8687
[x] Merge callbackList onto the updateQueue instead of a separate field. [@acdlite] #8728
[x] Separate priority level for state updates. #7457 [@acdlite]
[x] Fix coroutine issue where Fiber is passed in user space and could become stale. [@sebmarkbage] #8840
[x] DOM Dev Tools [@gaearon]

Phase 6: Unit tests and known bugs push

[x] [fb] Fix findDOMNode bug when used in certain cases from componentWillUnmount. [@sebmarkbage, @acdlite] #8897
[x] [fb] Provide ability to block event bubbling in portals [@spicyj]
[x] Unit tests: ReactFragment-test invariants [@acdlite] #8890
[x] Unit tests: ReactDOMProduction-test invariant [@acdlite] #8907
[x] Unit tests: ReactEmptyComponent-test invariant [@acdlite] #8908
[x] Ensure we replace throw new Error with invariant calls and they have sensible messages [@acdlite] #8926
[x] Unit tests: ReactMount-test [@acdlite] #8919
[x] Unit tests: ReactMultiChild-test (remove Map as children support in Stack) [@acdlite] #8911
[x] Unit tests: ReactMultiChildText-test [@acdlite]
[x] RN: Fix event bubbling regression. [@spicyj/@bvaughn]
[x] Make sure Babel doesn't generate bad code (e.g. IIFE for let/const), maybe fork Babel plugin to throw [@spicyj]
[x] RN: Land React update in fbsource with Fiber disabled [@sebmarkbage]
[x] Verify Fiber works in IE on Facebook (Map / Set: open IE10/11 and make sure it works) [@spicyj]
[x] [fb] Figure out Ads Image Cropper issue [@spicyj]
[x] Unit tests: ReactDOMComponent-test [@acdlite] #8948
[x] Unit tests: ReactComponentLifeCycle-test warnings (ex: fDN in render) [@acdlite] #8949
[x] Allow assigning this.state in cWRP with warning [@acdlite] #9040
[x] Unit tests: ReactStatelessComponent-test warning [@trueadm] #9043
[x] Unit tests: refs-test [@trueadm] #9045
[x] Make a new manual propTypes checker. [@acdlite] #9004
[x] Unit tests: ReactCompositeComponent-test warnings (ex: setState in render) [@acdlite] #8950
[x] Ensure "break on all/uncaught exceptions" works as expected in browsers [@acdlite] #8961
[x] Investigate what modules and unit tests fail if we remove Stack [@trueadm] #9069
[x] Quick investigation into FB require.js perf [@trueadm]
[x] Quick investigation into Fiber bundle perf, cutting out extra dev-only code [@trueadm] #9096
[x] [fb] Move .hidden check into hostConfig (https://fburl.com/m8juo29d, https://fburl.com/538fgu7g)

Phase 6.1: 15.5 Release

[ ] Warn (throw?) when doing an update on a container that was manually emptied outside of React (in stack, this mounted a brand-new tree; in fiber, it tries to apply an update and usually fails) [~~@keyanzhang~~, @sebmarkbage took over this] (blocks beta)
[x] Extract propTypes into a separate npm package. Get prop-types package name. [@acdlite]
[x] Unit tests: ReactContextValidator-test -- Pass the correct previous context to componentDidUpdate (or deprecate feature and remove test). [@bvaughn] #8631
[x] [fb] Better fix to ReactART.Text.
[x] Update warning when calling propTypes directly to refer user to checkPropTypes API
[x] Docs updates for test utils and removed add-ons [@flarnie]

Phase 6.2: React Native Fiber

[x] Reimplement Shallow Renderer without Stack dependencies [@lelandrichardson/@trueadm] #8982
[ ] RN: ensure we keep all important invariants (such as that text must be wrapped in <Text>) (does not block anything but we should do this)
[ ] Ensure that we call the FB specific warning module instead of the fbjs/lib/warning one in RN. [@spicyj] (doesn't block anything OSS, internal to FB)
[x] Convert propTypes callers to use checkPropTypes. [@acdlite]
[x] Deploy forwarding modules to react/lib/ReactCurrentOwner, react/lib/ReactComponentTreeHook, fbjs/lib/invariant, fbjs/lib/warning, fbjs/lib/ExecutionEnvironment, fbjs/lib/performanceNow, fbjs/lib/emptyObject, fbjs/lib/emptyFunction, fbjs/lib/shallowEqual. So that we can keep using providesModule in this repo until we have flat bundles. Or better yet: Do the inverse and switch to using CommonJS naming convention in www. [@sebmarkbage, @bvaughn]
[ ] See if we can codemod createStrictPropTypeChecker away and use exactShape instead. (doesn't block OSS release)
[x] Release a new version of react-redux whose peer is compatible with 16-alpha. Update RN to use it. https://github.com/reactjs/react-redux/pull/629
[x] Enable prepareNewChildrenBeforeUnmountInStack feature flag (one week after landing sync).
[x] ~~Codemod Flow errors yielded by properly typing findNodeHandle.~~ (we added types and $FlowFixMe comments; good enough for now)
[x] Redesign the host component "type", the type of host component refs and integration with NativeMethodsMixin to be upgrade compatible. [@bvaughn]
- [x] Unblocks: RN Dev Tools [@gaearon] https://github.com/facebook/react-devtools/pull/528
- [x] Unblocks: RN Inspector (see _known issues_ section of PR #8560) [@gaearon] https://github.com/facebook/react-native/pull/12421
[ ] Make sure instanceProps in ReactNativeComponentTree doesn't leak. (@sebmarkbage, @bvaughn)

Phase 6.3: Flat Bundles/Rollup

[x] Make React Perf work with Fiber (ReactPerf-test) [@gaearon] (Decided not to in favor of deeper timeline integration)
[x] Unit tests: ReactComponentTreeHook-test [@gaearon]
[x] Unit tests: ReactHostOperationHistoryHook-test [@gaearon]
[x] Make a list of all internal React modules required by FB code
[x] Come up with a better strategy for dealing with DEV code and make sure we don't ship it in prod
[x] Change www sync script https://fburl.com/juffnc9d to run rollup using commonjs source
- [x] Shimming the required files (ex: ReactDOMInjection, warning)
- [x] Exporting a single bundle that exposes the internals we need
- [x] Make fowarding modules for ReactDOM/etc as well as internal modules
[ ] Convert OSS repo to ES6 modules (does not block release; nice-to-have follow up)
[x] Make separate dev and non-dev build in one file for www (if (__DEV__) { ... all of React ... } else { ... all of React })
[x] Convert OSS repo to rollup
[x] Replace current Grunt/Gulp build system with a unified build system (maybe purely using Rollup)
[ ] Add Closure Compiler with ADVANCED and deal with mangling properly (does not block release; nice-to-have follow up)

Phase 7: Async-Compatible (does not block release)

[ ] Fix incremental regression (if it is a regression)
[ ] [fb] Fix resumeMountClassInstance so that newProps gets passed to construct class. Otherwise this.props can be null.
[ ] RN: Aborting async work or unmounting a tree due to an error leaks native views
[ ] Add cross-renderer support for portals (necessary for async ART)
[ ] ReactDOMFiber.render - Return the root instance from render even if priority is deferred. Test and cover incremental cases
[ ] Decide on replacing priorities with deadlines to fix starvation

Phase 8: Server-Side Rendering

[x] What are we doing?
[x] Unit tests: ReactRenderDocument-test (SSR) -- Reviving server rendered markup (including rendering into document and shadow DOM containers)

Phase 9: Improvements (does not block release)

[ ] The getPublicInstance internal API should consistently be used before exposing any stateNodes.
- [ ] See https://github.com/facebook/react/pull/8751#discussion_r119161506
[x] Key-warnings on fragments. [@flarnie]
[x] Don't mark "update" effect on newly mounted components unless they have a componentDidMount life-cycle.
[ ] Add support to Enzyme for Fiber (https://github.com/airbnb/enzyme/pull/1007)
[ ] Currently bailing out on equal props/state equality means that we still copy all of our children, and traverse all of them. Meaning that a parent with many children will still do a lot of work even if it bails out / "skips". If "pendingWorkPriority" on referred only to children, we would know if any children has additional work. [@acdlite] #8716
[x] Unit tests: ReactDOMTextComponent-test -- .normalize() case
[ ] Reconsider the ContentReset flag. Is there a cleaner solution for clearing out text content / innerHTML? Such as changing the insertion order
[ ] Reconsider storing masked contexts on instances
[ ] Revert inputValueTracking and replace with different comparison strategy. getter/setters is slow and complicated. Subscription model isn't helpful for Fiber's goals. The principle of over listening to lots of events and diffing the value is sound. ~~We haven't shipped in OSS yet so not too hard to fix~~ -- This will probably get pushed back to 16/17
[ ] Fiber drops the lazy DOM insertion order. Is there a solution that isn't slow in IE11? Is it worth fixing or should we leave IE11 slow and wait for a fix in the browser? (related: #8417)
[ ] Follow up from previous task (https://github.com/facebook/react/pull/8728#discussion_r95643611): use a linked list for callbackList [@acdlite] https://github.com/facebook/react/pull/8752
[ ] Ensure that it is possible to resume "incomplete" parents without rerendering. A shallow reuse. Store it on memoizedProps. Resuming "completed" parents is already possible. [@acdlite] #8716
[ ] Move callback invariant closer to usage as requested in https://github.com/facebook/react/pull/8639#issuecomment-271944608
[ ] Replace some invariants with warnings to help code size https://github.com/facebook/react/pull/8677#issuecomment-270335705

Source

sebmarkbage

👍89 ❤16

Most helpful comment

How to Help

Watch an intro to Fiber
Read about it here and here
Clone React repo
Run REACT_DOM_JEST_USE_FIBER=1 npm test to run unit tests with Fiber
Pick a failing test and try to fix it (unless it fits into a broader group claimed by someone in the first comment)
Run scripts/fiber/record-tests to verify which tests are now passing with Fiber
Submit a PR!

Note: Contributing is hard right now.

It will get easier once more pieces (e.g. DOM attributes and events) are in place. There will be a long trail of issues where community can really help. But if you're feeling adventurous, you can give it a go. Check out older Fiber PRs for inspiration and pieces of information. Example Fiber "easy" PR: https://github.com/facebook/react/pull/8156.

gaearon on 8 Nov 2016

👍44

All 10 comments

Things that I hope would also be appended to this list of features/bugs:

Fixing context
Making stateless functional components more performant
Learning from inferno to provide stateless function components some lifecycle hooks
Maybe revising/improving reacts legacy build step
Try to cut down react size in the major rewrite

donnieflorence on 30 Oct 2016

@donnieflorence This is a list of things we need to do to get parity with the existing implementation. After we finish these we might look at adding new features.

sophiebits on 30 Oct 2016

👍5

How to Help

Watch an intro to Fiber
Read about it here and here
Clone React repo
Run REACT_DOM_JEST_USE_FIBER=1 npm test to run unit tests with Fiber
Pick a failing test and try to fix it (unless it fits into a broader group claimed by someone in the first comment)
Run scripts/fiber/record-tests to verify which tests are now passing with Fiber
Submit a PR!

Note: Contributing is hard right now.

gaearon on 8 Nov 2016

👍44

will you support bind context like VueJS?. So there is no need bind function manually.

stefensuhat on 3 Feb 2017

@ssuhat

This is a list of things we need to do to get parity with the existing implementation. After we finish these we might look at adding new features.

https://github.com/facebook/react/issues/7925#issuecomment-257168615

acdlite on 3 Feb 2017

@ssuhat I think they explicitly decided against this a long time ago for being too "magical". Using the experimental property initializer syntax is a nice way to deal with it; you can also try a 3rd party plugin like react-autobind.

rchanou on 3 Feb 2017

Aside from being "magical," the problem with autobinding is that there's a performance cost to binding methods, and it's wasteful to autobind the ones that don't need it.

acdlite on 3 Feb 2017

Something akin to Inferno's linkEvent would be good though, skipping binding while allowing data to be passed through. Often I find myself with code in a loop that would benefit from this, e.g.

<ul>
  {things.map(thing => (
    <li key={thing.key} onClick={() => this.doSomething(thing.id)}>{thing.something}</li>
  ))}
</ul>

Being able to drop the binding there would be nice.

elyobo on 3 Feb 2017

Let's create a separate issue for any further discussions.

sebmarkbage on 3 Feb 2017

We just moved remaining open items from 6.1-6.3 into https://github.com/facebook/react/issues/8854 and for now decided to archive this issue. We can open a new 'umbrella' issue or individual issues for items in phases 7-9 after we finish the 16.0 release.