get update #2
Open
get update #2
Conversation
This file contains hidden or 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
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.)
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)
* 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>
* 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.
* Legacy context * Port Classes from Fiber to Fizz
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.
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.
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?
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.
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.
Undefined errors as a direct return value. This changes semantics for "true" and functions to mirror the client.
* Add enableSyncDefaultUpdates feature flag * Add enableSyncDefaultUpdates implementation * Fix tests * Switch feature flag to true by default * Finish concurrent render whenever for non-sync lanes * Also return DefaultLane with eventLane * Gate interruption test * Add continuout native event test * Fix tests from rebasing main * Hardcode lanes, remove added export * Sync forks
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.
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.
* Prevent reading canonical property of null * prettier
…r Replay integration) (#21032)
* Don't use default args * Hoist out creation for better inlining The closures prevent inlining otherwise.
* Clean up partial renderer entry points I made a mistake by leaving server.browser.stable in which is the partial renderer for the browser build of stable. That should use the legacy fizz one. Since the only usage of the partial renderer now is at FB and we don't use it with Node, I removed the Node build of partial renderer too. * Remove GC test No code is running this path anymore. Ideally this should be ported to a Fizz form.
* Use the server src files as entry points for the builds/tests We need one top level entry point to target two builds so we can't have the top level one be the entry point for the builds. * Same thing but with the modern entry point
This adds a new top level API for hydrating a root. It takes the initial children as part of its constructor. These are unlike other render calls in that they have to represent what the server sent and they can't be batched with other updates. I also changed the options to move the hydrationOptions to the top level since now these options are all hydration options. I kept the createRoot one just temporarily to make it easier to codemod internally but I'm doing a follow up to delete. As part of this I un-dried a couple of paths. ReactDOMLegacy was intended to be built on top of the new API but it didn't actually use those root APIs because there are special paths. It also doesn't actually use most of the commmon paths since all the options are ignored. It also made it hard to add only warnings for legacy only or new only code paths. I also forked the create/hydrate paths because they're subtly different since now the options are different. The containers are also different because I now error for comment nodes during hydration which just doesn't work at all but eventually we'll error for all createRoot calls. After some iteration it might make sense to break out some common paths but for now it's easier to iterate on the duplicates.
Upgrades the deprecation warning to a runtime error. I did it this way instead of removing the export so the type is the same in both builds. It will get dead code eliminated regardless.
Co-authored-by: Dan Abramov <dan.abramov@me.com>
These callsites were already removed as far as I can tell.
Currently, in a React 18 root, `act` only works if you mock the Scheduler package. This was because we didn't want to add additional checks at runtime. But now that the `act` testing API is dev-only, we can simplify its implementation. Now when an update is wrapped with `act`, React will bypass Scheduler entirely and push its tasks onto a special internal queue. Then, when the outermost `act` scope exists, we'll flush that queue. I also removed the "wrong act" warning, because the plan is to move `act` to an isomorphic entry point, simlar to `startTransition`. That's not directly related to this PR, but I didn't want to bother re-implementing that warning only to immediately remove it. I'll add the isomorphic API in a follow up. Note that the internal version of `act` that we use in our own tests still depends on mocking the Scheduler package, because it needs to work in production. I'm planning to move that implementation to a shared (internal) module, too.
* Move internal version of act to shared module No reason to have three different copies of this anymore. I've left the the renderer-specific `act` entry points because legacy mode tests need to also be wrapped in `batchedUpdates`. Next, I'll update the tests to use `batchedUpdates` manually when needed. * Migrates tests to use internal module directly Instead of the `unstable_concurrentAct` exports. Now we can drop those from the public builds. I put it in the jest-react package since that's where we put our other testing utilities (like `toFlushAndYield`). Not so much so it can be consumed publicly (nobody uses that package except us), but so it works with our build tests. * Remove unused internal fields These were used by the old act implementation. No longer needed.
Change format of @next and @experimental release versions from <number>-<sha> to <number>-<sha>-<date> to make them more human readable. This format still preserves the ability for us to easily map a version number to the changes it contains, while also being able to more easily know at a glance how recent a release is.
* Add tests for invokeGuardedCallback * Add skipped failing tests * Check next render works * Mirror tests for createRoot * Move comments around
This prevents it from buffering adn suppressing all console logs until a test has completed running (When debugging in Chrome).
When wrapping an update in act, instead of scheduling a microtask, we can add the task to our internal queue. The benefit is that the user doesn't have to await the act call. We can flush the work synchronously. This doesn't account for microtasks that are scheduled in userspace, of course, but it at least covers React's usage.
* Enable skipped tests from #21723 * Report uncaught errors in DEV * Clear caught error This is not necessary (as proven by tests) because next invokeGuardedCallback clears it anyway. But I'll keep it for consistency with other calls.
* Add tests for disabled logs * Always keep disabled logs in the second pass * Jest nit * Always use the second result
* Move error logging to update callback This prevents double logging for gDSFE boundaries with createRoot. * Add an explanation for the rest of duplicates
Some new ones had slipped in (e.g. deprecated ReactDOM.render message from 18)
Test for fix added in #21740
When migrating some internal tests I found it annoying that I couldn't
return anything from the `act` scope. You would have to declare the
variable on the outside then assign to it. But this doesn't play well
with type systems — when you use the variable, you have to check
the type.
Before:
```js
let renderer;
act(() => {
renderer = ReactTestRenderer.create(<App />);
})
// Type system can't tell that renderer is never undefined
renderer?.root.findByType(Component);
```
After:
```js
const renderer = await act(() => {
return ReactTestRenderer.create(<App />);
})
renderer.root.findByType(Component);
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
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.
Before submitting a pull request, please make sure the following is done:
master.yarnin the repository root.yarn test). Tip:yarn test --watch TestNameis helpful in development.yarn test-prodto test in the production environment. It supports the same options asyarn test.yarn debug-test --watch TestName, openchrome://inspect, and press "Inspect".yarn prettier).yarn lint). Tip:yarn lincto only check changed files.yarn flow).Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html