* [eslint-plugin-react-cooks] Report constant constructions
The dependency array passed to a React hook can be thought of as a list of cache keys. On each render, if any dependency is not `===` its previous value, the hook will be rerun. Constructing a new object/array/function/etc directly within your render function means that the value will be referentially unique on each render. If you then use that value as a hook dependency, that hook will get a "cache miss" on every render, making the dependency array useless.
This can be especially dangerous since it can cascade. If a hook such as `useMemo` is rerun on each render, not only are we bypassing the option to avoid potentially expensive work, but the value _returned_ by `useMemo` may end up being referentially unique on each render causing other downstream hooks or memoized components to become deoptimized.
* Fix/remove existing tests
* Don't give an autofix of wrapping object declarations
It may not be safe to just wrap the declaration of an object, since the object may get mutated.
Only offer this autofix for functions which are unlikely to get mutated.
Also, update the message to clarify that the entire construction of the value should get wrapped.
* Handle the long tail of nodes that will be referentially unique
* Catch let/var constant constructions on initial assignment
* Trim trailing whitespace
* Address feedback from @gaearon
* Rename "assignment" to "initialization"
* Add test for a constant construction used in multiple dependency arrays
Because `postTask` returns a promise, errors inside a `postTask`
callback result in the promise being rejected.
If we don't catch those errors, then the browser will report an
"Unhandled promise rejection" error. This is a confusing message to see
in the console, because the fact that `postTask` is a promise-based API
is an implementation detail from the perspective of the developer.
"Promise rejection" is a red herring.
On the other hand, if we do catch those errors, then we need to report
the error to the user in some other way.
What we really want is the default error reporting behavior that a
normal, non-Promise browser event gets.
So, we'll re-throw inside `setTimeout`.
This updates the experimental Scheduler postTask build to call postTask
directly, instead of managing our own custom queue and work loop.
We still use a deadline 5ms mechanism to implement `shouldYield`.
The main thing that postTask is currently missing is the continuation
feature — when yielding to the main thread, the yielding task is sent
to the back of the queue, instead of maintaining its position.
While this would be nice to have, even without it, postTask may be good
enough to replace our userspace implementation.
We'll run some tests to see.
* Add ReactVersion to SchedulingProfiler render scheduled marks
* Move ReactVersion to a new --react-init-* mark
Co-authored-by: E-Liang Tan <eliang@eliangtan.com>
* Support inner component _debugOwner in memo
* test with devtool context
* remove memo test
* Merged master; tweaked test and snapshot
* Pass owner to createFiber fn when creating a memo component.
Co-authored-by: Theodore Han <tqhan317@gmail.com>
* test: Simulate mouseover in browser
* Fix duplicate onMouseEnter event when relatedTarget is a root
* Test leave as well
Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
* test: Add current behavior for event types of onFocus/onBlur
* fix: onFocus/onBlur have a matching event type
* fix useFocus
* fix: don't compare native event types with react event types
* Add FocusIn/FocusOutEventInterface
* A simpler alternative fix
* Add regression tests
* Always pass React event type and fix beforeinput
Co-authored-by: Dan Abramov <dan.abramov@me.com>
A passive effect's cleanup function may throw after an unmount. In that event, React sometimes threw an uncaught runtime error trying to access a property on a null stateNode field. This commit fixes that (and adds a regression test).
* Get current time from performance.now in non-DOM environments
* Use local references to native APIs for Date and Performance
* Refactored to read globals directly
* Test: Don't unmount siblings of deleted node
Adds a failing regression test. Will fix in the next commit.
* Refactor to accept deleted fiber, not child list
A deleted fiber is passed to
flushPassiveUnmountEffectsInsideOfDeletedTree, but the code is written
as if it accepts the first node of a child list. This is likely because
the function was based on similar functions like
`flushPassiveUnmountEffects`, which do accept a child list.
Unfortunately, types don't help here because we use the first node
in the list to represent the whole list, so in both cases, the type
is Fiber.
Might be worth changing the other functions to also accept individual
fibers instead of a child list, to help avoid confusion.
* Add layout effect to regression test, just in case
Creates new subtree tag, PassiveStatic, that represents whether a
tree contains any passive effect hooks.
It corresponds to the PassiveStatic effect tag, which represents the
same concept for an individual fiber.
This allows us to remove the PassiveStatic effect tag from PassiveMask.
Its presence was causing us to schedule a passive effect phase callback
on every render, instead of only when something changed. That's now
fixed; this is reflected in the SchedulerProfiler tests.
(The naming is getting really confusing. Need to do some bikeshedding.)
* setCurrentFiber per fiber, instead of per effect
* Re-use safelyCallDestroy
Part of the code in flushPassiveUnmountEffects is a duplicate of the
code used for unmounting layout effects. I did some minor refactoring to
so we could use the same function in both places.
Closure will inline anyway so it doesn't affect code size or
performance, just maintainability.
* Don't check HookHasEffect during deletion
We don't need to check HookHasEffect during a deletion; all effects are
unmounted.
So we also don't have to set HookHasEffect during a deletion, either.
This allows us to remove the last remaining passive effect logic from
the synchronous layout phase.
* Remove opaque event type
* Rename type and merge files
* Use literals where we have Flow coverage
* Flowify some plugins
* Remove constants except necessary ones
The root fiber doesn't have a parent from which we can read the
`subtreeTag`, so we need to check its `effectTag` directly.
The root fiber previously did not have any pending passive effects,
but it does now that deleted fibers are cleaned up in the passive phase.
This allows us to remove a `schedulePassiveEffectCallback` call from the
synchronous unmount path.
Co-authored-by: Brian Vaughn <bvaughn@fb.com>
Saves us from having to set a flag on `current` during the layout phase.
Could result in some redundant traversal, since PassiveStatic includes
effects that don't need clean-up. But it's worth it to remove the work
from the layout phase.
While I was editing this, I also re-arranged it so that we check the
`effectTag` check before we check the `tag`, since the `effectTag` check
is the one that's more likely to fail.
* Adds new `Passive` subtree tag value.
* Adds recursive traversal for passive effects (mounts and unmounts).
* Removes `pendingPassiveHookEffectsMount` and `pendingPassiveHookEffectsUnmount` arrays from work loop.
* Re-adds sibling and child pointer detaching (temporarily removed in previous PR).
* Addresses some minor TODO comments left over from previous PRs.
---
Co-authored-by: Luna Ruan <luna@fb.com>
Clay Tercek
Jordan Eldredge
Sebastian Markbåge
Brian Vaughn
inottn
Andrew Clark
Pascal Fong Kye
Ricky
Dan Abramov
Kartik Choudhary
Dan Abramov
Sebastian Silbermann
Gustavo Saiani
Mateusz Burzyński
DaniAcu
Dominic Gannaway
Emilis Baliukonis
Halit Ogunc