Skip to content

don't use gql in hook.js#1287

Merged
phryneas merged 4 commits intomainfrom
pr/hook-no-gql
Mar 22, 2024
Merged

don't use gql in hook.js#1287
phryneas merged 4 commits intomainfrom
pr/hook-no-gql

Conversation

@phryneas
Copy link
Copy Markdown
Member

@phryneas phryneas commented Mar 21, 2024

This moves the usage of gql out of the injected hook.js into the DevTools panel.

That should significantly reduce the amount of injected code.

image
🎉

@phryneas phryneas requested a review from a team as a code owner March 21, 2024 12:54
@relativeci
Copy link
Copy Markdown

relativeci bot commented Mar 21, 2024

Job #115: Bundle Size — 1.09MiB (-3.14%).

d69e6d7(current) vs ec7e332 main#114(baseline)

Warning

Bundle contains 5 duplicate packages – View duplicate packages

Bundle metrics  Change 5 changes Improvement 3 improvements
                 Current
Job #115
     Baseline
Job #114
Improvement  Initial JS 1.05MiB(-3.25%) 1.09MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 89.81% 4.52%
No change  Chunks 5 5
No change  Assets 12 12
Change  Modules 491(-4.47%) 514
Improvement  Duplicate Modules 29(-44.23%) 52
Improvement  Duplicate Code 4.28%(-47.61%) 8.17%
No change  Packages 60 60
No change  Duplicate Packages 4 4
Bundle size by type  Change 1 change Improvement 1 improvement
                 Current
Job #115
     Baseline
Job #114
Improvement  JS 1.05MiB (-3.25%) 1.09MiB
Not changed  IMG 35.85KiB 35.85KiB
Not changed  HTML 810B 810B
Not changed  Other 778B 778B

View job #115 reportView pr/hook-no-gql branch activityView project dashboard

Comment on lines -3 to -5
export type JSONPrimitive = boolean | null | string | number;
export type JSONObject = { [key in string]?: JSONValue };
export type JSONValue = JSONPrimitive | JSONValue[] | JSONObject;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There were two slightly different definitions of that in the codebase - I retired this one.

name: "ExplorerRequest";
operationId: string;
operation: string;
variables: JSONValue;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is definitely never an array or a primitive value ;)

Comment on lines +52 to +59
panelWindow.send({
type: "explorerRequest",
payload: {
operation: gql(operation),
operationName,
variables,
fetchPolicy,
},
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Actually, no need to stringify this one.

Everything here already came in from a postMessage, so it's transportable. The only possible exception would be the DocumentNode itself, but that seems to work nicely with structuredClone.

operation: event.data.operation,
operationName: event.data.operationName,
variables: event.data.variables,
variables: event.data.variables ?? undefined,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

replacing a potential null with undefined.

Copy link
Copy Markdown
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.

Mind adding a changeset for this one? Otherwise this looks great! What a great size savings here. Simple change but super effective 🚀

@phryneas phryneas merged commit 052f242 into main Mar 22, 2024
@phryneas phryneas deleted the pr/hook-no-gql branch March 22, 2024 09:30
@github-actions github-actions bot mentioned this pull request Mar 22, 2024
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.

2 participants