My personal workflow is to develop against the www-modern release
channel, with the variant flags enabled, because it encompasses the
largest set of features. Then I rely on CI to run the tests against
all the other configurations.
So in practice, I almost always run
```
yarn test -r=www-modern --variant TEST_FILE
```
instead of
```
yarn test TEST_FILE
```
So, I've updated the `yarn test` command to use those options
by default.
I recently started using `pendingPassiveEffectsLanes` to check if there were any pending
passive effects (530027a). `pendingPassiveEffectsLanes` is the value of
`root.finishedLanes` at the beginning of the commit phase. When there
are pending passive effects, it should always be a non-zero value,
because it represents the lanes used to render the effects.
But it turns out that `root.finishedLanes` isn't always correct.
Sometimes it's `NoLanes` even when there's a new commit.
I found this while investigating an internal bug report. The only repro
I could get was via a headless e2e test runner; I couldn't get one in an
actual browser, or other interactive environment. I used the e2e test to
bisect and confirm the fix. But I don't know yet know how to write a
regression test for the precise underlying scenario. I can probably
reverse engineer one by studying the code; after a quick glance
at `performConcurrentWorkOnRoot` and `performSyncWorkOnRoot`, it's not
hard to see how this might happen.
In the meantime, I'll revert the recent change that exposed the bug.
I was surprised that this had never come up before, since the code that
assigns `root.finishedLanes` is in an extremely hot path, and it hasn't
changed in a while. The reason is that, before 530027a,
`root.finishedLanes` was only used by the DevTools profiler, which is
probably why we had never noticed any issues. In addition to fixing the
inconsistency, we might also consider making `finishedLanes` a
profiling-only field.
We assume that isArray and getIteratorFn are only called on objects.
So we shouldn't have to check that again and again, and then check a flag.
We can just stay in this branch.
There is a slight semantic breakage here because you could have an
iterator on a function, such as if it's a generator function. But that's
not supported and that currently only works at the root. The inner slots
don't support this.
So this just makes it consistent.
Tracked Fibers are called "updaters" and are exposed to DevTools via a 'memoizedUpdaters' property on the ReactFiberRoot. The implementation of this feature follows a vaguely similar approach as interaction tracing, but does not require reference counting since there is no subscriptions API.
This change is in support of a new DevTools Profiler feature that shows which Fiber(s) scheduled the selected commit in the Profiler.
All changes have been gated behind a new feature flag, 'enableUpdaterTracking', which is enabled for Profiling builds by default. We also only track updaters when DevTools has been detected, to avoid doing unnecessary work.
I recently added UI for the Profiler's commit and post-commit durations to the DevTools, but I made two pretty silly oversights:
1. I used the commit hook (called after mutation+layout effects) to read both the layout and passive effect durations. This is silly because passive effects may not have flushed yet git at this point.
2. I didn't reset the values on the HostRoot node, so they accumulated with each commit.
This commitR addresses both issues:
1. First it adds a new DevTools hook, onPostCommitRoot*, to be called after passive effects get flushed. This gives DevTools the opportunity to read passive effect durations (if the build of React being profiled supports it).
2. Second the work loop resets these durations (on the HostRoot) after calling the post-commit hook so address the accumulation problem.
I've also added a unit test to guard against this regressing in the future.
* Doing this in flushPassiveEffectsImpl seemed simplest, since there are so many places we flush passive effects. Is there any potential problem with this though?
I screwed this up in #21082. Got confused by the < versus > thing again.
The helper functions are annoying, too, because I always forget the
intended order of the arguments. But they're still helpful because when
we refactor the type we only have the change the logic in one place.
Added a regression test.
We've just initialized the update queue above this and there's no user
code that executes between.
The general API that prevents this from mattering is that you can't
call setState in the constructor.
* Split out into helper functions
This is similar to the structure of beginWork in Fiber.
* Split the rendering of a node from recursively rendering a node
This lets us reuse render node at the root which doesn't spawn new work.
* Remove redundant initial of isArray (#21163)
* Reapply prettier
* Type the isArray function with refinement support
This ensures that an argument gets refined just like it does if isArray is
used directly.
I'm not sure how to express with just a direct reference so I added a
function wrapper and confirmed that this does get inlined properly by
closure compiler.
* A few more
* Rename unit test to internal
This is not testing a bundle.
Co-authored-by: Behnam Mohammadi <itten@live.com>
We have been building DevTools to target Chrome 49 and Firefox 54. These are super old browser versions and they did not have full ES6 support, so the generated build is more bloated than it needs to be.
DevTools uses most modern language features. Off the top of my head, we it uses basically everything but async and generator functions.
Based on CanIUse charts– I believe that in order to avoid unnecessary polyfill/wrapper code being generated, we'd need to target Chrome 60+ (released 2017-07-25) and Firefox 55+ (released 2017-04-18). This seems like a reasonable set of browsers to target.
Note that we can't remove the IE 11 target from the react-devtools-core backend yet due to Hermes (React Native) ES6 support but that should be doable by the end of the year given current engineering targets. But we could update the frontend target, as well as the targets for the extensions and the react-devtools-inline package.
This commit increases the browser targets then for Chrome (from 49 to 60) and Firefox (from 54 to 55)
This commit contains a proposed change to layout effect semantics within Suspense subtrees: If a component mounts within a Suspense boundary and is later hidden (because of something else suspending) React will cleanup that component’s layout effects (including React-managed refs).
This change will hopefully fix existing bugs that occur because of things like reading layout in a hidden tree and will also enable a point at which to e.g. pause videos and hide user-managed portals. After the suspended boundary resolves, React will setup the component’s layout effects again (including React-managed refs).
The scenario described above is not common. The useTransition API should ensure that Suspense does not revert to its fallback state after being mounted.
Note that these changes are primarily written in terms of the (as of yet internal) Offscreen API as we intend to provide similar effects semantics within recently shown/hidden Offscreen trees in the future. (More to follow.)
(Note that all changes in this PR are behind a new feature flag, enableSuspenseLayoutEffectSemantics, which is disabled for now.)
Scheduler's heap implementation sometimes accesses indices that are out
of bounds (larger than the size of the array). This causes a VM de-opt.
This change fixes the de-opt by always checking the index before
accessing the array. In exchange, we can remove the typecheck on the
returned element.
Background: https://v8.dev/blog/elements-kinds#avoid-reading-beyond-the-length-of-the-array
Co-authored-by: Andrew Clark <git@andrewclark.io>
* DevTools: useModalDismissSignal bugfix
Make useModalDismissSignal's manually added click/keyboard events more robust to sync flushed passive effects. (Don't let the same click event that shows a modal dialog also dismiss it.)
* Replaced event.timeStamp check with setTimeout
* Implement DOM format config structure
* Styles
* Input warnings
* Textarea special cases
* Select special cases
* Option special cases
We read the currently selected value from the FormatContext.
* Warning for non-lower case HTML
We don't change to lower case at runtime anymore but keep the warning.
* Pre tags innerHTML needs to be prefixed
This is because if you do the equivalent on the client using innerHTML,
this is the effect you'd get.
* Extract errors
If a discrete render results in passive effects, we should flush them
synchronously at the end of the current task so that the result is
immediately observable. For example, if a passive effect adds an event
listener, the listener will be added before the next input.
We don't need to do this for effects that don't have discrete/sync
priority, because we assume they are not order-dependent and do not
need to be observed by external systems.
For legacy mode, we will maintain the existing behavior, since it hasn't
been reported as an issue, and we'd have to do additional work to
distinguish "legacy default sync" from "discrete sync" to prevent all
passive effects from being treated this way.
* Support nesting of startTransition and flushSync
* Unset transition before entering any special execution contexts
Co-authored-by: Andrew Clark <git@andrewclark.io>
* [Fast Refresh] Support callthrough HOCs
* Add a newly failing testing to demonstrate the flaw
This shows why my initial approach doesn't make sense.
* Attach signatures at every nesting level
* Sign nested memo/forwardRef too
* Add an IIFE test
This is not a case that is important for Fast Refresh, but we shouldn't change the code semantics. This case shows the transform isn't quite correct. It's wrapping the call at the wrong place.
* Find HOCs above more precisely
This fixes a false positive that was causing an IIFE to be wrapped in the wrong place, which made the wrapping unsafe.
* Be defensive against non-components being passed to setSignature
* Fix lint
* Add onError option to Flight Server
The callback is called any time an error is generated in a server component.
This allows it to be logged on a server if needed. It'll still be rethrown
on the client so it can be logged there too but in case it never reaches
the client, here's a way to make sure it doesn't get lost.
* Add fatal error handling