experimental `ApolloClient.getMemoryInternals` helper #11409

phryneas · 2023-12-04T15:53:10Z

Opening this one for feedback, and to see how feasible it is to have a helper like this.

Is this possible without adding a lot of bulk to the prod bundle size?

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

…ache

…uery-weakCache

changeset-bot · 2023-12-04T15:53:14Z

🦋 Changeset detected

Latest commit: c3ad505

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@apollo/client	Patch

Not sure what this means? Click here to learn what changesets are.

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

github-actions · 2023-12-04T15:56:48Z

size-limit report 📦

Path	Size
dist/apollo-client.min.cjs	37.9 KB (+0.11% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	45.52 KB (+2.28% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production)	43.06 KB (+0.19% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	33.92 KB (+2.53% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production)	31.85 KB (+0.18% 🔺)
import { ApolloProvider } from "dist/react/index.js"	1.24 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production)	1.22 KB (0%)
import { useQuery } from "dist/react/index.js"	5.2 KB (+16.74% 🔺)
import { useQuery } from "dist/react/index.js" (production)	4.27 KB (0%)
import { useLazyQuery } from "dist/react/index.js"	5.51 KB (+15.62% 🔺)
import { useLazyQuery } from "dist/react/index.js" (production)	4.58 KB (+0.03% 🔺)
import { useMutation } from "dist/react/index.js"	3.51 KB (+27.75% 🔺)
import { useMutation } from "dist/react/index.js" (production)	2.73 KB (0%)
import { useSubscription } from "dist/react/index.js"	3.19 KB (+31.4% 🔺)
import { useSubscription } from "dist/react/index.js" (production)	2.39 KB (-0.05% 🔽)
import { useSuspenseQuery } from "dist/react/index.js"	5.18 KB (+16.46% 🔺)
import { useSuspenseQuery } from "dist/react/index.js" (production)	3.85 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js"	4.77 KB (+18.36% 🔺)
import { useBackgroundQuery } from "dist/react/index.js" (production)	3.43 KB (0%)
import { useLoadableQuery } from "dist/react/index.js"	5 KB (+17.25% 🔺)
import { useLoadableQuery } from "dist/react/index.js" (production)	3.67 KB (0%)
import { useReadQuery } from "dist/react/index.js"	3.01 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production)	2.95 KB (0%)
import { useFragment } from "dist/react/index.js"	2.11 KB (0%)
import { useFragment } from "dist/react/index.js" (production)	2.06 KB (0%)

alessbell · 2023-12-13T15:43:44Z

src/utilities/caching/getMemoryInternals.ts

+ */
+export const getApolloClientMemoryInternals =
+  __DEV__ ?
+    (_getApolloClientMemoryInternals as RemoveThis<


For future me, a comment about RemoveThis would be helpful here. Something like:

Since InMemoryCache inherits from ApolloCache, TS complains about the incompatibility of this, and so while it's useful to define the type of this as either InMemoryCache or ApolloCache for the purpose of the actual implementations in _getApolloCacheMemoryInternals and _getInMemoryCacheMemoryInternals, we remove this before exporting to avoid the TS conflict on assignment to their respective prototypes.

alessbell · 2023-12-13T15:53:26Z

src/utilities/caching/getMemoryInternals.ts

+      ].filter((x) => x != null);
+}
+
+// removeTypenameFromVariables getVariableDefinitions


Can remove :D

alessbell

Excited to integrate this into devtools 🔥

jerelmiller

Most of my comments were pretty minor and nitpicky. Super stoked about this though! This will be nice to display in the dev tools.

jerelmiller · 2023-12-13T22:40:08Z

src/utilities/caching/__tests__/getMemoryInternals.ts

+            persistedQueryHashes: 1,
+          },
+          {
+            getVariableDefinitions: 0,


I'd be curious if you'd be interested in a sort of namespacing or identification for these links. Since these memory profiles will be useful for dev tools, its going to be really difficult to correlate these numbers with the links they belong to. Perhaps something like this?

Suggested change

getVariableDefinitions: 0,

'removeTypenameFromVariables.getVariableDefinitions': 0,

or

Suggested change

getVariableDefinitions: 0,

removeTypenameFromVariables: {

getVariableDefinitions: 0,

}

or maybe it makes sense to add a name property 🤔

Suggested change

getVariableDefinitions: 0,

name: 'removeTypenameFromVariables',

getVariableDefinitions: 0,

Thoughts? Or does this clutter this up too much?

Let's wait for the pattern we come up over in the cache PR then we'll adjust here :)

jerelmiller · 2023-12-13T22:42:36Z

src/utilities/caching/getMemoryInternals.ts

+        link?.getMemoryInternals?.(),
+        ...linkInfo(link?.left),
+        ...linkInfo(link?.right),
+      ].filter((x) => x != null);


Suggested change

].filter((x) => x != null);

].filter(Boolean);

might also be a nice shortcut that saves a couple bytes 🙂.

I had that, but it also removed 0 values in the DocumentTransform use case further down (which just returns a number, not an object).
So we definitely need this down there. I can see if extracting it out can save a few bytes so at least we don't repeat the function definition. (Although we are talking dev bundle size anyways ^^)

Results in the same size, but slightly nicer: e283de3

src/utilities/caching/getMemoryInternals.ts

src/utilities/graphql/DocumentTransform.ts

jerelmiller · 2023-12-13T22:52:52Z

src/utilities/caching/getMemoryInternals.ts

+      },
+      links: linkInfo(this.link),
+      queryManager: {
+        Transforms: this["queryManager"]["transformCache"].size,


Suggested change

Transforms: this["queryManager"]["transformCache"].size,

transformCache: this["queryManager"]["transformCache"].size,

I'm curious on the decision to name this as Transforms while the rest of the entries are lowercase and typically named after the property they are pulling from. Would it make more sense to call this transformCache?

The related cache configuration option is currently called queryManagerTransforms, so it seemed to make sense to have this as queryManager.Transforms instead of a completely different name.

src/utilities/caching/__tests__/getMemoryInternals.ts

src/link/core/ApolloLink.ts

jerelmiller

💯

netlify · 2023-12-15T14:59:05Z

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	`94d1b85`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/657c68904e4e9d00084b493d
😎 Deploy Preview	https://deploy-preview-11409--apollo-client-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

netlify · 2023-12-15T15:04:05Z

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	`d84a05b`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/657c691974f6d90008ce3008
😎 Deploy Preview	https://deploy-preview-11409--apollo-client-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

src/utilities/caching/getMemoryInternals.ts

phryneas added 24 commits November 16, 2023 18:09

print: use WeakCache instead of WeakMap

d5d8cb1

format

29bf59d

pull in memory testing tools from PR 11358

9942e61

Persisted Query Link: improve memory management

b933dfb

re-add accidentally removed dependency

a117bd0

update api

0a1b718

Merge remote-tracking branch 'origin/release-3.9' into pr/print-weakC…

d64eee1

…ache

update size limit

bd40e8e

Merge branch 'release-3.9' into pr/print-weakCache

c712446

size-limit

f84da80

Merge branch 'pr/print-weakCache' into pr/persisted-query-weakCache

7485662

Merge branch 'release-3.9' into pr/persisted-query-weakCache

5361820

fix test failure

6ad8e56

better cleanup of interval/timeout

7cbf8dc

Merge remote-tracking branch 'origin/release-3.9' into pr/persisted-q…

d609c67

…uery-weakCache

apply formatting

0521529

remove unneccessary type

188dd0a

format again after updating prettier

4283908

add central confiuguration for Apollo Client cache sizes

9d3bd68

resolve import cycle

825e012

add exports

3762c5e

reduce cache collection throttle timeout

554240c

typo in comment

af8b16f

experimental ApolloClient.getCacheStatus helper

6ce2613

phryneas added this to the MemoryAnalysis milestone Dec 4, 2023

phryneas added 3 commits December 4, 2023 17:17

update size-limits

147a491

fix circular import

324e4c9

size-limits

2e65620

alessbell reviewed Dec 13, 2023

View reviewed changes

alessbell approved these changes Dec 13, 2023

View reviewed changes

github-actions bot added the auto-cleanup 🤖 label Dec 13, 2023

phryneas and others added 2 commits December 13, 2023 19:01

PR feedback, long explanatory comment

9f3db9c

Clean up Prettier, Size-limit, and Api-Extractor

ade170d

jerelmiller approved these changes Dec 13, 2023

View reviewed changes

phryneas added 4 commits December 14, 2023 12:19

adjust comments

ea82c2e

remove expect.objectContaining

61c80a3

flip conditionals

2ccd229

extract function

e283de3

jerelmiller approved these changes Dec 14, 2023

View reviewed changes

Base automatically changed from pr/cache-sizes to release-3.9 December 15, 2023 09:52

phryneas and others added 2 commits December 15, 2023 15:53

Merge remote-tracking branch 'origin/release-3.9' into pr/getCacheStatus

94d1b85

Clean up Prettier, Size-limit, and Api-Extractor

d84a05b

phryneas and others added 4 commits December 15, 2023 16:15

report correct size limits

55cb419

Clean up Prettier, Size-limit, and Api-Extractor

c7a65aa

adjust report shape to roughly match configuration

4283e22

Clean up Prettier, Size-limit, and Api-Extractor

47534c8

phryneas commented Dec 15, 2023

View reviewed changes

src/utilities/caching/getMemoryInternals.ts Show resolved Hide resolved

phryneas and others added 4 commits December 15, 2023 16:51

Update src/utilities/caching/getMemoryInternals.ts

3fcab50

Clean up Prettier, Size-limit, and Api-Extractor

a4c4aae

better types

6f85640

Clean up Prettier, Size-limit, and Api-Extractor

c3ad505

phryneas merged commit 2e7203b into release-3.9 Dec 15, 2023
26 checks passed

phryneas deleted the pr/getCacheStatus branch December 15, 2023 16:15

github-actions bot mentioned this pull request Dec 15, 2023

Version Packages (beta) #11417

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

experimental `ApolloClient.getMemoryInternals` helper #11409

experimental `ApolloClient.getMemoryInternals` helper #11409

phryneas commented Dec 4, 2023

changeset-bot bot commented Dec 4, 2023 •

edited

Loading

github-actions bot commented Dec 4, 2023 •

edited

Loading

alessbell Dec 13, 2023

alessbell Dec 13, 2023

alessbell left a comment

jerelmiller left a comment

jerelmiller Dec 13, 2023

phryneas Dec 14, 2023

jerelmiller Dec 13, 2023

phryneas Dec 14, 2023 •

edited

Loading

phryneas Dec 14, 2023

jerelmiller Dec 13, 2023

phryneas Dec 14, 2023

jerelmiller left a comment

netlify bot commented Dec 15, 2023

netlify bot commented Dec 15, 2023

	getVariableDefinitions: 0,
	'removeTypenameFromVariables.getVariableDefinitions': 0,

	getVariableDefinitions: 0,
	name: 'removeTypenameFromVariables',
	getVariableDefinitions: 0,

	Transforms: this["queryManager"]["transformCache"].size,
	transformCache: this["queryManager"]["transformCache"].size,

experimental ApolloClient.getMemoryInternals helper #11409

experimental ApolloClient.getMemoryInternals helper #11409

Conversation

phryneas commented Dec 4, 2023

Checklist:

changeset-bot bot commented Dec 4, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Dec 4, 2023 • edited Loading

size-limit report 📦

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alessbell left a comment

Choose a reason for hiding this comment

jerelmiller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phryneas Dec 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerelmiller left a comment

Choose a reason for hiding this comment

netlify bot commented Dec 15, 2023

✅ Deploy Preview for apollo-client-docs ready!

netlify bot commented Dec 15, 2023

✅ Deploy Preview for apollo-client-docs ready!

experimental `ApolloClient.getMemoryInternals` helper #11409

experimental `ApolloClient.getMemoryInternals` helper #11409

changeset-bot bot commented Dec 4, 2023 •

edited

Loading

github-actions bot commented Dec 4, 2023 •

edited

Loading

phryneas Dec 14, 2023 •

edited

Loading