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

Upgrade to urql v4 under the hood #171

Merged
merged 1 commit into from
Jun 26, 2023
Merged

Upgrade to urql v4 under the hood #171

merged 1 commit into from
Jun 26, 2023

Conversation

airhorns
Copy link
Contributor

@airhorns airhorns commented Apr 17, 2023

This upgrades us to urql v4 for all generated api client packages.

  • removes the deprecated exchanges that are built into the client now to get rid of that pesky message
  • adds the new mandatory properties to the OperationResult type

More comments inline

Gadget side test PR here: https://github.com/gadget-inc/gadget/pull/6756 (it works!)

@@ -100,7 +99,7 @@ export class GadgetConnection {
} else {
this._fetchImplementation = fetchPolyfill;
}
this.websocketImplementation = options.websocketImplementation ?? WebSocket;
this.websocketImplementation = options.websocketImplementation ?? globalThis?.WebSocket ?? WebSocket;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also upgraded the isomorphic-ws library, which stopped monkeypatching the global. This makes our tests keep working.

@@ -328,20 +328,21 @@ export class GadgetConnection {
}

private newBaseClient() {
const exchanges = [dedupExchange];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

urql 4 doesnt use this exchange anymore

return {
subscribe: (sink) => {
const dispose = this.baseSubscriptionClient.subscribe(operation, sink as Sink<ExecutionResult>);
const input = { ...request, query: request.query || "" };
const dispose = this.baseSubscriptionClient.subscribe(input, sink as Sink<ExecutionResult>);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This type changed between versions of urql -- the query is now options for some reason, so this maps to the kind of object that graphql-ws is expecting

@@ -19,9 +19,9 @@
"prerelease": "gitpkg publish"
},
"dependencies": {
"@gadgetinc/api-client-core": "^0.13.10",
"@gadgetinc/api-client-core": "^0.13.14",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our react bindings now need the newest api-client-core which also depends on urql 4, as well as depending on urql 4 themselves

Copy link
Contributor

@ngc ngc left a comment

Choose a reason for hiding this comment

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

api-client-core upgrade with docs tests passing

🥳

@airhorns airhorns force-pushed the urql-4 branch 3 times, most recently from d41a4f9 to 8be6b1f Compare June 26, 2023 01:17
@airhorns
Copy link
Contributor Author

ping @jasong689 💖

 - removes the deprecated exchanges that are built into the client now
 - adds the new mandatory properties to the `OperationResult` type
@airhorns airhorns merged commit cb27206 into main Jun 26, 2023
3 checks passed
@airhorns airhorns deleted the urql-4 branch June 26, 2023 15:47
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.

3 participants