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

useSuspenseQuery: remove promiseCache #11363

Merged
merged 12 commits into from
Dec 1, 2023

Conversation

phryneas
Copy link
Member

Also looking at the other suspense hooks, but putting this here already so @jerelmiller can take a look.

Checklist:

  • If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

Copy link

changeset-bot bot commented Nov 15, 2023

⚠️ No Changeset found

Latest commit: 3f5b734

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 Nov 15, 2023

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 37.77 KB (-0.05% 🔽)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 44.26 KB (-0.02% 🔽)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 42.73 KB (-0.06% 🔽)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 32.87 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 31.55 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.28 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.26 KB (0%)
import { useQuery } from "dist/react/index.js" 4.38 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.19 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 4.69 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.51 KB (0%)
import { useMutation } from "dist/react/index.js" 2.65 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.63 KB (0%)
import { useSubscription } from "dist/react/index.js" 2.34 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 2.29 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 4.33 KB (-0.85% 🔽)
import { useSuspenseQuery } from "dist/react/index.js" (production) 3.75 KB (-0.96% 🔽)
import { useBackgroundQuery } from "dist/react/index.js" 3.86 KB (-0.21% 🔽)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.26 KB (-0.18% 🔽)
import { useLoadableQuery } from "dist/react/index.js" 4.11 KB (-0.19% 🔽)
import { useLoadableQuery } from "dist/react/index.js" (production) 3.52 KB (-0.2% 🔽)
import { useReadQuery } from "dist/react/index.js" 3.05 KB (-0.07% 🔽)
import { useReadQuery } from "dist/react/index.js" (production) 3 KB (+0.07% 🔺)
import { useFragment } from "dist/react/index.js" 2.15 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.1 KB (0%)

@phryneas phryneas added this to the MemoryAnalysis milestone Nov 16, 2023
@phryneas phryneas force-pushed the pr/remove-useSuspenseQuery-promiseCache branch from 79577b5 to b8c51af Compare November 16, 2023 11:21
we only care about identity, not about contents
Copy link
Member

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good to me. Thanks for digging into this further and making some improvements here. Sometimes I forget that a new empty object is enough to be a unique identifier in a lot of cases, rather than the "original" thing. Thanks!

@@ -92,7 +92,6 @@ export class InternalQueryReference<TData = unknown> {
// Don't save this result as last result to prevent delivery of last result
// when first subscribing
this.result = observable.getCurrentResult(false);
this.key = options.key;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the key is no longer needed from options, could you remove this from InternalQueryReferenceOptions and stop passing it from SuspenseCache?

src/react/hooks/useSuspenseQuery.ts Show resolved Hide resolved
@phryneas phryneas marked this pull request as ready for review November 30, 2023 17:26
@phryneas phryneas merged commit aaf8c79 into release-3.9 Dec 1, 2023
22 checks passed
@phryneas phryneas deleted the pr/remove-useSuspenseQuery-promiseCache branch December 1, 2023 10:32
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants