Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove a bunch of act calls in useSuspenseQuery test #11918

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented Jul 1, 2024

Since this test suite is extremely slow in React 19, I want to try if removing act where possible speeds it up in any way.

Copy link

changeset-bot bot commented Jul 1, 2024

⚠️ No Changeset found

Latest commit: edc76e1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Jul 1, 2024

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 38.68 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 47.48 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 45.05 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 34.22 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 32.08 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.26 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.24 KB (0%)
import { useQuery } from "dist/react/index.js" 5.32 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.39 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 5.6 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.67 KB (0%)
import { useMutation } from "dist/react/index.js" 3.59 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.81 KB (0%)
import { useSubscription } from "dist/react/index.js" 3.23 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 2.43 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 5.47 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 4.12 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 4.98 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.63 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" 5.04 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production) 3.69 KB (0%)
import { useReadQuery } from "dist/react/index.js" 3.35 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 3.3 KB (0%)
import { useFragment } from "dist/react/index.js" 2.32 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.27 KB (0%)

Copy link

netlify bot commented Jul 1, 2024

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit edc76e1
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/66828ac63a1c6b000815306e
😎 Deploy Preview https://deploy-preview-11918--apollo-client-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@phryneas
Copy link
Member Author

phryneas commented Jul 1, 2024

With this PR:

% hyperfine --runs 5 -- 'yarn test useSuspenseQuery --selectProjects "ReactDOM 18" --silent'
Benchmark 1: yarn test useSuspenseQuery --selectProjects "ReactDOM 18" --silent
  Time (mean ± σ):     10.244 s ±  1.850 s    [User: 6.877 s, System: 0.606 s]
  Range (min … max):    9.359 s … 13.551 s    5 runs
% hyperfine --runs 5 -- 'yarn test useSuspenseQuery --selectProjects "ReactDOM 19" --silent'
Benchmark 1: yarn test useSuspenseQuery --selectProjects "ReactDOM 19" --silent
  Time (mean ± σ):     68.795 s ±  1.287 s    [User: 9.550 s, System: 0.811 s]
  Range (min … max):   68.105 s … 71.078 s    5 runs

On main:

% hyperfine --runs 5 -- 'yarn test useSuspenseQuery --selectProjects "ReactDOM 18" --silent'          main
Benchmark 1: yarn test useSuspenseQuery --selectProjects "ReactDOM 18" --silent
  Time (mean ± σ):      9.385 s ±  0.907 s    [User: 6.011 s, System: 0.599 s]
  Range (min … max):    8.907 s … 11.004 s    5 runs
% hyperfine --runs 5 -- 'yarn test useSuspenseQuery --selectProjects "ReactDOM 19" --silent'          main
Benchmark 1: yarn test useSuspenseQuery --selectProjects "ReactDOM 19" --silent
  Time (mean ± σ):     77.302 s ±  1.008 s    [User: 14.714 s, System: 1.897 s]
  Range (min … max):   76.564 s … 78.775 s    5 runs

So, this shaves off about 10s, but does not make the big difference.

@phryneas
Copy link
Member Author

phryneas commented Jul 1, 2024

I did some further investigation, and there are a few test impacted by this. I have very small sample sizes, so some of these numbers might be hiccups.

Tests that significantly changed from bad change to good change (positive numbers bad, negative numbers good)

NAME PR improvement 19 React 19 with PR compared to React 18 main
useSuspenseQuery skip option works with startTransition 92 100
useSuspenseQuery fetchMore works with startTransition when setting errorPolicy as default option in ApolloClient constructor 79 368
useSuspenseQuery works with startTransition to change variables 72 381
useSuspenseQuery fetchMore works with startTransition to allow React to show stale UI until finished suspending 70 359
useSuspenseQuery skipToken works with startTransition when used for options 65 70
useSuspenseQuery applies updated fetchPolicy on next fetch when it changes between renders 57 349
useSuspenseQuery fetchMore does not cause extra render 57 357
useSuspenseQuery properly resolves when skip becomes false when returning a result that is deeply equal to data in the cache 56 640
useSuspenseQuery tears down the query if the component never renders again after suspending 36 53
useSuspenseQuery properly resolves refetch when returning a result that is deeply equal to data in the cache 30 627
useSuspenseQuery properly handles changing options along with changing variables 27 619
-- -- --
useSuspenseQuery re-suspends the component when changing queries and using a "network-only" fetch policy -69 626
-- -- --
useSuspenseQuery validates the GraphQL query as a query -131 -7
useSuspenseQuery refetch works with startTransition to allow React to show stale UI until finished suspending -183 290
useSuspenseQuery applies context on next fetch when it changes between renders -248 371
useSuspenseQuery applies errorPolicy on next fetch when it changes between renders -252 361
useSuspenseQuery returns errors after calling refetch when errorPolicy is set to "all" -256 379
useSuspenseQuery defaults refetchWritePolicy to "overwrite" -263 359
useSuspenseQuery throws errors when errors are returned after calling refetch -300 315
useSuspenseQuery tears down subscription when throwing an error on refetch -360 312
useSuspenseQuery does not oversubscribe when suspending multiple times -551 412
useSuspenseQuery works with useDeferredValue -5821 390

The last one got so much better because it no longer triggers the React bug(?) we reported here:
facebook/react#29855

So while this PR gives some improvements overall, tests just got a lot slower, even with these changes.

How much worse did it get?
Let's see the worst offenders (everything that got slower by over 300ms):

NAME Main 18 Main19
useSuspenseQuery works with useDeferredValue 79 6290
useSuspenseQuery does not oversubscribe when suspending multiple times 14 977
useSuspenseQuery refetches and suspends when switching back to already used variables while using network-only fetch policy 14 953
useSuspenseQuery re-suspends multiple times when calling refetch multiple times 13 947
useSuspenseQuery refetches and suspends when switching back to already used variables while using no-cache fetch policy 14 946
useSuspenseQuery applies changed refetchWritePolicy to next fetch when changing between renders 35 947
useSuspenseQuery returns merged array when fetchMore returns empty array of results 194 1035
useSuspenseQuery re-suspends the component when changing queries and using a "network-only" fetch policy 10 705
useSuspenseQuery tears down subscription when throwing an error on refetch 10 682
useSuspenseQuery re-suspends the component when changing queries and using a "no-cache" fetch policy 10 665
useSuspenseQuery re-suspends the component when changing queries and using a "cache-first" fetch policy 11 648
useSuspenseQuery re-suspends the component when changing queries and using a "cache-and-network" fetch policy 23 658
useSuspenseQuery returns errors after calling refetch when errorPolicy is set to "all" 9 644
useSuspenseQuery handles partial data results after calling refetch when errorPolicy is set to "all" 11 641
useSuspenseQuery uses cached result and does not suspend when switching back to already used variables while using cache-first fetch policy 13 643
useSuspenseQuery incrementally rerenders data returned by a refetch for a deferred query 118 747
useSuspenseQuery re-suspends when calling refetch 8 637
useSuspenseQuery ensures data is fetched the correct amount of times when changing variables and using a "cache-and-network" fetch policy 8 637
useSuspenseQuery ensures data is fetched the correct amount of times when changing variables and using a "network-only" fetch policy 9 638
useSuspenseQuery responds to cache updates after changing variables 63 691
useSuspenseQuery responds to cache updates after changing back to already fetched variables 65 693
useSuspenseQuery re-suspends the component when changing variables and using a "no-cache" fetch policy 11 639
useSuspenseQuery suspends and fetches data from new client when changing clients 8 635
useSuspenseQuery suspends when changing variables 10 637
useSuspenseQuery can refetch and respond to cache updates after encountering an error in an incremental chunk for a deferred query when errorPolicy is all 169 795
useSuspenseQuery clears errors when changing variables and errorPolicy is set to "all" 11 637
useSuspenseQuery re-suspends the component when changing variables and using a "cache-and-network" fetch policy 11 636
useSuspenseQuery re-suspends the component when changing variables and using a "network-only" fetch policy 13 636
useSuspenseQuery tears down all queries when rendering with multiple variable sets 14 637
useSuspenseQuery defaults refetchWritePolicy to "overwrite" 22 644
useSuspenseQuery uses cached result with network request and does not suspend when switching back to already used variables while using cache-and-network fetch policy 63 684
useSuspenseQuery rerenders data returned by fetchMore for a deferred query 70 689
useSuspenseQuery applies context on next fetch when it changes between renders 8 627
useSuspenseQuery tears down all queries when multiple clients are used 13 631
useSuspenseQuery ensures data is fetched the correct amount of times when changing variables and using a "no-cache" fetch policy 8 626
useSuspenseQuery honors refetchWritePolicy set to "merge" 21 639
useSuspenseQuery honors refetchWritePolicy set to "overwrite" 21 638
useSuspenseQuery throws errors when errors are returned after calling refetch 11 626
useSuspenseQuery re-suspends when calling refetch with new variables 12 626
useSuspenseQuery applies errorPolicy on next fetch when it changes between renders 10 623
useSuspenseQuery ensures data is fetched the correct amount of times when changing variables and using a "cache-first" fetch policy 23 632
useSuspenseQuery re-suspends the component when changing variables and using a "cache-first" fetch policy 19 627
useSuspenseQuery re-suspends when calling fetchMore with different variables 32 640
useSuspenseQuery properly uses cache field policies when calling fetchMore without updateQuery 32 639
useSuspenseQuery properly uses updateQuery when calling fetchMore 32 630
useSuspenseQuery properly resolves refetch when returning a result that is deeply equal to data in the cache 39 636
useSuspenseQuery properly resolves fetchMore when returning a result that is deeply equal to data in the cache 39 635
useSuspenseQuery properly handles changing options along with changing variables 31 623
useSuspenseQuery properly resolves when skip becomes false when returning a result that is deeply equal to data in the cache 47 631
useSuspenseQuery allows custom query key so two components that share same query and variables do not interfere with each other 57 630
useSuspenseQuery refetch works with startTransition to allow React to show stale UI until finished suspending 37 510
useSuspenseQuery suspends and does not use partial data when changing variables and using a "cache-first" fetch policy with returnPartialData 60 404
useSuspenseQuery responds to cache updates and clears errors after an error returns when errorPolicy is set to "all" 60 385
useSuspenseQuery suspends when switching away from skipToken in options 5 329
useSuspenseQuery does not make network requests when skip is true 5 329
useSuspenseQuery renders skip result, does not suspend, and maintains data when skipping a query with skipToken as options after it was enabled 5 328
useSuspenseQuery returns partial data results and discards GraphQL errors when errorPolicy is set to "ignore" 5 328
useSuspenseQuery does not respond to cache updates when using a "no-cache" fetch policy 108 431
useSuspenseQuery does not throw or return graphql errors when errorPolicy is set to "ignore" 4 326
useSuspenseQuery returns the same results for the same variables 10 331
useSuspenseQuery merges global default variables with local variables 8 329
useSuspenseQuery does not make network requests when using skipToken for options 5 325
useSuspenseQuery uses default variables from the client when none provided in options in strict mode 5 324
useSuspenseQuery suspends when refetching after returning cached data for the initial fetch 4 323
useSuspenseQuery suspends when partial data is in the cache and using a "no-cache" fetch policy with returnPartialData 5 324
useSuspenseQuery suspends deferred queries with lists and properly patches results 111 430
useSuspenseQuery discards partial data and throws errors returned in incremental chunks 11 329
useSuspenseQuery maintains results when rerendering a query using a "no-cache" fetch policy 5 321
useSuspenseQuery does not make network requests when using skip with strict mode 5 321
useSuspenseQuery responds to cache updates when using a "cache-first" fetch policy 58 374
useSuspenseQuery ensures data is fetched and suspended the correct amount of times in strict mode while using a "no-cache" fetch policy 4 320
useSuspenseQuery incrementally rerenders data returned by a fetchMore for a deferred query 1064 1379
useSuspenseQuery ensures refetch, fetchMore, and subscribeToMore are referentially stable even after result data has changed 57 372
useSuspenseQuery returns partial data and keeps errors when errorPolicy is set to "all" 5 320
useSuspenseQuery handles multiple graphql errors when errorPolicy is set to "all" 7 322
useSuspenseQuery persists errors between rerenders when errorPolicy is set to "all" 5 319
useSuspenseQuery skip result is referentially stable when using skipToken as options 5 319
useSuspenseQuery responds to cache updates in strict mode while using a "network-only" fetch policy 58 372
useSuspenseQuery suspends a query with variables and returns results 11 325
useSuspenseQuery applies returnPartialData on next fetch when it changes between renders 163 476
useSuspenseQuery can subscribe to subscriptions and react to cache updates via subscribeToMore 60 373
useSuspenseQuery skip result is referentially stable 6 319
useSuspenseQuery ensures data is fetched and suspended the correct amount of times in strict mode while using a "network-only" fetch policy 4 317
useSuspenseQuery uses the default fetch policy from the client when none provided in options 5 318
useSuspenseQuery renders skip result, does not suspend, and maintains data when skip becomes true after it was false 7 320
useSuspenseQuery suspends deferred queries until initial chunk loads then streams in data as it loads when using a "cache-first" fetch policy 58 371
useSuspenseQuery suspends and does not use partial data when changing variables and using a "cache-and-network" fetch policy with returnPartialData 60 373
useSuspenseQuery returns the client used in the result 3 316
useSuspenseQuery responds to cache updates and clears errors after an error returns when errorPolicy is set to "ignore" 58 370
useSuspenseQuery uses default variables from the client when none provided in options 5 317
useSuspenseQuery can unset a globally defined variable 4 316
useSuspenseQuery adds partial data and does not throw errors returned in incremental chunks but returns them in error property with errorPolicy set to all 60 372
useSuspenseQuery suspends deferred queries until initial chunk loads then streams in data as it loads when using a "cache-and-network" fetch policy 60 371
useSuspenseQuery discards multiple graphql errors when errorPolicy is set to "ignore" 4 315
useSuspenseQuery writes to the cache when using a "cache-and-network" fetch policy 7 318
useSuspenseQuery ensures result is referentially stable 13 323
useSuspenseQuery suspends deferred queries until initial chunk loads then streams in data as it loads when using a "network-only" fetch policy 59 369
useSuspenseQuery responds to cache updates in strict mode while using a "cache-first" fetch policy 58 368
useSuspenseQuery ensures data is fetched and suspended the correct amount of times in strict mode while using a "cache-and-network" fetch policy 4 314
useSuspenseQuery tears down the query on unmount 7 317
useSuspenseQuery writes to the cache when using a "network-only" fetch policy 7 317
useSuspenseQuery works with startTransition to change variables 48 357
useSuspenseQuery does not make network requests when using skipToken with strict mode 6 315
useSuspenseQuery suspends when skip becomes false after it was true 5 314
useSuspenseQuery responds to cache updates in strict mode while using a "cache-and-network" fetch policy 58 367
useSuspenseQuery does not write to the cache when using a "no-cache" fetch policy 6 315
useSuspenseQuery suspends deferred queries until initial chunk loads then streams in data as it loads when using a "no-cache" fetch policy 60 369
useSuspenseQuery suspends and does not overwrite cache when data is in the cache and using a "no-cache" fetch policy 5 314
useSuspenseQuery allows the client to be overridden in strict mode 5 313
useSuspenseQuery responds to cache updates when using a "network-only" fetch policy 58 366
useSuspenseQuery responds to cache updates when using a "cache-and-network" fetch policy 60 368
useSuspenseQuery throws graphql errors by default 6 314
useSuspenseQuery incrementally renders data returned after skipping a deferred query 61 368
useSuspenseQuery passes context to the link 3 310
useSuspenseQuery suspends deferred queries until initial chunk loads then streams in data as it loads 60 367
useSuspenseQuery suspends and refetches data when part of the query key changes 49 356
useSuspenseQuery allows the client to be overridden 4 310
useSuspenseQuery ensures data is fetched and suspended the correct amount of times in strict mode while using a "cache-first" fetch policy 4 310
useSuspenseQuery suspends queries with deferred fragments in lists and properly merges arrays 61 366
useSuspenseQuery writes to the cache when using a "cache-first" fetch policy 7 312
useSuspenseQuery suspends when partial data is in the cache and using a "network-only" fetch policy with returnPartialData 7 312
useSuspenseQuery throws network errors when errorPolicy is set to "none" 7 312
useSuspenseQuery ignores errors returned after calling refetch when errorPolicy is set to "ignore" 7 311
useSuspenseQuery suspends when partial data is in the cache and using a "cache-first" fetch policy 7 311
useSuspenseQuery suspends when data is in the cache and using a "network-only" fetch policy 4 308
useSuspenseQuery handles multiple graphql errors when errorPolicy is set to "none" 7 310
useSuspenseQuery throws graphql errors when errorPolicy is set to "none" 6 307
useSuspenseQuery fetchMore does not cause extra render 142 442

@phryneas phryneas marked this pull request as draft July 1, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant