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

Use queryId as id for queries #1438

Open
wants to merge 7 commits into
base: jerel/query-status
Choose a base branch
from

Conversation

jerelmiller
Copy link
Member

Switches the value of id used for queries to the queryId its associated with in Apollo Client rather than its index in the map. This should allow us to build additional functionality in the future that targets the proper query rather than guessing based on its index in the map.

@jerelmiller jerelmiller requested a review from a team as a code owner July 1, 2024 00:30
Copy link

relativeci bot commented Jul 1, 2024

#652 Bundle Size — 1.29MiB (+0.8%).

ffc1b85(current) vs 5b9e30d main#644(baseline)

Warning

Bundle contains 13 duplicate packages – View duplicate packages

Bundle metrics  Change 4 changes Regression 1 regression
                 Current
#652
     Baseline
#644
Regression  Initial JS 1.25MiB(+0.82%) 1.24MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 94.93% 0%
No change  Chunks 5 5
No change  Assets 12 12
Change  Modules 1040(+1.17%) 1028
No change  Duplicate Modules 49 49
Change  Duplicate Code 3.92%(-0.51%) 3.94%
No change  Packages 162 162
No change  Duplicate Packages 10 10
Bundle size by type  Change 1 change Regression 1 regression
                 Current
#652
     Baseline
#644
Regression  JS 1.25MiB (+0.82%) 1.24MiB
No change  IMG 35.85KiB 35.85KiB
No change  HTML 810B 810B
No change  Other 778B 778B

Bundle analysis reportBranch jerel/use-query-idProject dashboard

@@ -46,6 +46,7 @@ export interface SerializedError {
}

export type QueryInfo = {
Copy link
Member

Choose a reason for hiding this comment

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

Could we rename this type at some point? It's a bit of a red herring since we have a type of the same name over in AC.

Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

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.

None yet

2 participants