facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.1k stars 46.87k forks source link

[Fiber] Umbrella for remaining features / bugs #7925

Closed sebmarkbage closed 7 years ago

sebmarkbage commented 8 years ago

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](https://gist.github.com/342ebd8df0d08143381dd38c01d13d15). #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

Phase 6.2: React Native Fiber

Phase 6.3: Flat Bundles/Rollup

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

Phase 8: Server-Side Rendering

Phase 9: Improvements (does not block release)

donnieflorence commented 8 years ago

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

  1. Fixing context
  2. Making stateless functional components more performant
  3. Learning from inferno to provide stateless function components some lifecycle hooks
  4. Maybe revising/improving reacts legacy build step
  5. Try to cut down react size in the major rewrite
sophiebits commented 8 years ago

@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.

gaearon commented 8 years ago

How to Help

  1. Watch an intro to Fiber
  2. Read about it here and here
  3. Clone React repo
  4. Run REACT_DOM_JEST_USE_FIBER=1 npm test to run unit tests with Fiber
  5. Pick a failing test and try to fix it (unless it fits into a broader group claimed by someone in the first comment)
  6. Run scripts/fiber/record-tests to verify which tests are now passing with Fiber
  7. 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.

stefensuhat commented 7 years ago

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

acdlite commented 7 years ago

@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

rchanou commented 7 years ago

@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.

acdlite commented 7 years ago

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.

elyobo commented 7 years ago

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.

sebmarkbage commented 7 years ago

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

flarnie commented 7 years ago

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.