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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions packages/api-client-core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@gadgetinc/api-client-core",
"version": "0.13.13",
"version": "0.13.14",
"source": "src/index.ts",
"main": "dist/src/index.js",
"types": "dist/src/index.d.ts",
Expand All @@ -20,16 +20,15 @@
},
"dependencies": {
"@opentelemetry/api": "^1.4.0",
"@urql/core": "^3.0.1",
"@urql/exchange-multipart-fetch": "^1.0.1",
"cross-fetch": "^3.0.6",
"@urql/core": "^4.0.10",
"cross-fetch": "^3.1.5",
"gql-query-builder": "^3.7.2",
"graphql": "~16.6.0",
"graphql-ws": "^5.5.5",
"isomorphic-ws": "^4.0.1",
"graphql-ws": "^5.13.1",
"isomorphic-ws": "^5.0.0",
"lodash.clonedeep": "^4.5.0",
"lodash.isequal": "^4.5.0",
"ws": "^8.11.0"
"ws": "^8.13.0"
},
"devDependencies": {
"@types/lodash.clonedeep": "^4.5.6",
Expand Down
30 changes: 30 additions & 0 deletions packages/api-client-core/spec/support.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ describe("support utilities", () => {
{
operation: null as any,
data: { foo: { bar: "baz" } },
stale: false,
hasNext: false,
},
["foo", "bar"]
)
Expand All @@ -29,6 +31,8 @@ describe("support utilities", () => {
operation: null as any,
data: null,
error: new CombinedError({ networkError: new Error("foobar") }),
stale: false,
hasNext: false,
},

["foo", "bar"]
Expand All @@ -43,6 +47,8 @@ describe("support utilities", () => {
operation: null as any,
data: null,
error: new CombinedError({ networkError: new Error("foobar") }),
stale: false,
hasNext: false,
},

["foo", "bar"]
Expand All @@ -60,6 +66,8 @@ describe("support utilities", () => {
data: null,
// @ts-expect-error an array of network errors doesn't match urql's types, but we've observed it at runtime
error: new CombinedError({ networkError: [new Error("foo"), new Error("foo")] }),
stale: false,
hasNext: false,
},

["foo", "bar"]
Expand All @@ -77,6 +85,8 @@ describe("support utilities", () => {
operation: null as any,
data: null,
error: new CombinedError({ graphQLErrors: [new Error("foo")] }),
stale: false,
hasNext: false,
},

["foo", "bar"]
Expand All @@ -91,6 +101,8 @@ describe("support utilities", () => {
operation: null as any,
data: null,
error: new CombinedError({ graphQLErrors: [new Error("foo"), "bar"] }),
stale: false,
hasNext: false,
},

["foo", "bar"]
Expand All @@ -108,6 +120,8 @@ describe("support utilities", () => {
operation: null as any,
data: null,
error: new CombinedError({ graphQLErrors: [new GraphQLError("inner graphql error")] }),
stale: false,
hasNext: false,
},

["foo", "bar"]
Expand All @@ -122,6 +136,8 @@ describe("support utilities", () => {
operation: null as any,
data: null,
error: new CombinedError({ response: { whatever: true } }),
stale: false,
hasNext: false,
},

["foo", "bar"]
Expand All @@ -138,6 +154,8 @@ describe("support utilities", () => {
operation: null as any,
data: null,
error: new CombinedError({ graphQLErrors: [new GraphQLError("inner graphql error")] }),
stale: false,
hasNext: false,
},
["foo", "bar"]
)
Expand All @@ -150,6 +168,8 @@ describe("support utilities", () => {
operation: null as any,
data: undefined,
error: undefined,
stale: false,
hasNext: false,
},
["foo", "bar"]
);
Expand Down Expand Up @@ -194,6 +214,8 @@ describe("support utilities", () => {
{
operation: null as any,
data: { createWidget: { success: true, errors: null, widget: { bar: "baz" } } },
stale: false,
hasNext: false,
},
["createWidget"]
)
Expand All @@ -212,6 +234,8 @@ describe("support utilities", () => {
widget: null,
},
},
stale: false,
hasNext: false,
},

["createWidget"]
Expand All @@ -231,6 +255,8 @@ describe("support utilities", () => {
widget: null,
},
},
stale: false,
hasNext: false,
},

["createWidget"]
Expand Down Expand Up @@ -266,6 +292,8 @@ describe("support utilities", () => {
widget: null,
},
},
stale: false,
hasNext: false,
},
["createWidget"]
);
Expand Down Expand Up @@ -302,6 +330,8 @@ describe("support utilities", () => {
widget: null,
},
},
stale: false,
hasNext: false,
},
["createWidget"]
);
Expand Down
19 changes: 10 additions & 9 deletions packages/api-client-core/src/GadgetConnection.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { ClientOptions, RequestPolicy } from "@urql/core";
import { Client, cacheExchange, dedupExchange, subscriptionExchange } from "@urql/core";
import { multipartFetchExchange } from "@urql/exchange-multipart-fetch";
import { Client, cacheExchange, fetchExchange, subscriptionExchange } from "@urql/core";
import fetchPolyfill from "cross-fetch";
import type { ExecutionResult } from "graphql";
import type { Sink, Client as SubscriptionClient, ClientOptions as SubscriptionClientOptions } from "graphql-ws";
Expand Down Expand Up @@ -101,7 +100,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.

this.websocketsEndpoint = options.websocketsEndpoint ?? options.endpoint + "/batch";
this.websocketsEndpoint = this.websocketsEndpoint.replace(/^http/, "ws");
this.environment = options.environment ?? "Development";
Expand Down Expand Up @@ -215,10 +214,11 @@ export class GadgetConnection {
requestPolicy: "network-only", // skip any cached data during transactions
exchanges: [
subscriptionExchange({
forwardSubscription(operation) {
forwardSubscription(request) {
const input = { ...request, query: request.query || "" };
return {
subscribe: (sink) => {
const dispose = subscriptionClient!.subscribe(operation, sink as Sink<ExecutionResult>);
const dispose = subscriptionClient!.subscribe(input, sink as Sink<ExecutionResult>);
return {
unsubscribe: dispose,
};
Expand Down Expand Up @@ -329,20 +329,21 @@ export class GadgetConnection {
}

private newBaseClient() {
const exchanges = [dedupExchange, urlParamExchange];
const exchanges = [urlParamExchange];

// apply urql's default caching behaviour when client side (but skip it server side)
if (typeof window != "undefined") {
exchanges.push(cacheExchange);
}

exchanges.push(
multipartFetchExchange,
fetchExchange,
subscriptionExchange({
forwardSubscription: (operation) => {
forwardSubscription: (request) => {
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

return {
unsubscribe: dispose,
};
Expand Down
8 changes: 4 additions & 4 deletions packages/react/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@gadgetinc/react",
"version": "0.12.5",
"version": "0.12.6",
"source": "src/index.ts",
"main": "dist/src/index.js",
"types": "dist/src/index.d.ts",
Expand All @@ -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

"deep-equal": "^2.2.0",
"urql": "^3.0.3"
"urql": "^4.0.4"
},
"devDependencies": {
"@gadgetinc/api-client-core": "workspace:*",
Expand All @@ -30,7 +30,7 @@
"@types/lodash": "^4.14.191",
"@types/react": "^18.2.9",
"@types/react-dom": "^18.2.4",
"@urql/core": "^3.0.1",
"@urql/core": "^4.0.10",
"conditional-type-checks": "^1.0.6",
"graphql": "16.5.0",
"lodash": "^4.17.21",
Expand Down
14 changes: 14 additions & 0 deletions packages/react/spec/useAction.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ describe("useAction", () => {
},
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down Expand Up @@ -173,6 +175,8 @@ describe("useAction", () => {
},
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down Expand Up @@ -216,6 +220,8 @@ describe("useAction", () => {
},
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down Expand Up @@ -254,6 +260,8 @@ describe("useAction", () => {
},
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down Expand Up @@ -289,6 +297,8 @@ describe("useAction", () => {
},
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down Expand Up @@ -336,6 +346,8 @@ describe("useAction", () => {
},
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down Expand Up @@ -364,6 +376,8 @@ describe("useAction", () => {
},
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/react/spec/useBulkAction.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ describe("useBulkAction", () => {
],
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down Expand Up @@ -132,6 +134,8 @@ describe("useBulkAction", () => {
widgets: null,
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down Expand Up @@ -169,6 +173,8 @@ describe("useBulkAction", () => {
],
},
},
stale: false,
hasNext: false,
});

await act(async () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/react/spec/useFindBy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ describe("useFindBy", () => {
},
},
},
stale: false,
hasNext: false,
});

expect(result.current[0].data!.id).toEqual("123");
Expand Down Expand Up @@ -87,6 +89,8 @@ describe("useFindBy", () => {
},
},
},
stale: false,
hasNext: false,
});

expect(result.current[0].data).toBeFalsy();
Expand Down Expand Up @@ -115,6 +119,8 @@ describe("useFindBy", () => {
},
},
},
stale: false,
hasNext: false,
});

const data = result.current[0].data;
Expand Down
Loading