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

feat: add multipart subscription network adapters for Relay and urql #11301

Merged
merged 7 commits into from Nov 2, 2023

Conversation

alessbell
Copy link
Member

@alessbell alessbell commented Oct 17, 2023

This PR adds network layer adapters for Relay and urql to be able to consume multipart subscriptions.

Usage looks like this:

Relay

import { createFetchMultipartSubscription } from "@apollo/client/utilities/subscriptions/relay";
import { Environment, Network, RecordSource, Store } from "relay-runtime";

const fetchMultipartSubs = createFetchMultipartSubscription(
  "http://localhost:4000"
);

const network = Network.create(fetchQuery, fetchMultipartSubs);

export const RelayEnvironment = new Environment({
  network,
  store: new Store(new RecordSource()),
});

Urql

import { createFetchMultipartSubscription } from "@apollo/client/utilities/subscriptions/urql";
import { Client, fetchExchange, subscriptionExchange } from "@urql/core";

const url = "http://localhost:4000";

const multipartSubscriptionForwarder = createFetchMultipartSubscription(
  url
);

const client = new Client({
  url,
  exchanges: [
    fetchExchange,
    subscriptionExchange({
      forwardSubscription: multipartSubscriptionForwarder,
    }),
  ],
});

To start, the API surface is small: users must specify the uri and can also provide a fetch override and additional headers.

Testing

I've done manual testing with both Relay + urql, and I'd like to write some E2E tests in https://github.com/apollographql/client-router-e2e-tests since it's the integration with the respective frameworks that we care about here. I can open a follow-on PR there if that sounds good.

Documentation

Will do a follow-on PR for documentation.

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

@changeset-bot
Copy link

changeset-bot bot commented Oct 17, 2023

🦋 Changeset detected

Latest commit: 78f3230

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 Minor

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

};
const options = generateOptionsForMultipartSubscription(headers || {});

return Observable.create((sink) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

zen-observable-ts's Observable is incompatible with Relay's networking API here, so I'm using the relay-runtime's Observable. (Using the former produces a network.execute(...).do is not a function error.)

A related note about project dependencies: I added @types/relay-runtime to our dev dependencies, but did not specify relay-runtime as an optional peer dependency. The risk of confusing a lot of people seemed to outweigh the chance at a mild annoyance if someone were to try to drop in this subscriptions network layer without already having relay-runtime installed :)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2023

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 37.17 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 43.64 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 42.11 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 32.69 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 31.36 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.23 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.21 KB (0%)
import { useQuery } from "dist/react/index.js" 4.29 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.11 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 4.6 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.42 KB (0%)
import { useMutation } from "dist/react/index.js" 2.54 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.53 KB (0%)
import { useSubscription } from "dist/react/index.js" 2.24 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 2.2 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 4.29 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 3.73 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 3.77 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.21 KB (0%)
import { useReadQuery } from "dist/react/index.js" 3 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 2.95 KB (0%)
import { useFragment } from "dist/react/index.js" 2.1 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.05 KB (0%)


const backupFetch = maybe(() => fetch);

export function createFetchMultipartSubscription(
Copy link
Member Author

Choose a reason for hiding this comment

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

I used the same function name here but maybe this should be a more urql-y createMultipartSubscriptionForwarder

@alessbell alessbell changed the title feat: add multipart subscription adapters for Relay and urql feat: add multipart subscription network adapters for Relay and urql Oct 17, 2023
@alessbell
Copy link
Member Author

Updated the entrypoints and added them to config/entryPoints.js in 807e2cd.

I'm still seeing "risky cross-entry-point nested import" warnings: CleanShot 2023-11-01 at 16 32 18

But when I installed the npm pack-ed library in a project using the urql adapter and look at the generated bundles, they contain what I'd expect/function how I'd expect: CleanShot 2023-11-01 at 16 28 01

Thoughts on whether those warnings are safe to ignore here, @benjamn?

@benjamn
Copy link
Member

benjamn commented Nov 1, 2023

@alessbell Yes, I think those warnings are safe to ignore here.

The main point of the warnings is that code might be duplicated between our internal CommonJS bundles if you reach across the bundles to import nested files, but for these adapters that's not a problem, since consumers will be importing just the code they need, from a single entry point. Also, the warnings are not a big deal if you're using the ESM files rather than CommonJS.

const currentFetch = preferredFetch || maybe(() => fetch) || backupFetch;
const observerNext = sink.next.bind(sink);

currentFetch!(uri, options)
Copy link
Member

@phryneas phryneas Nov 2, 2023

Choose a reason for hiding this comment

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

We don't need to pass in an accept header here?

(Forget it I'm blind... generateOptionsForMultipartSubscription)

@phryneas
Copy link
Member

phryneas commented Nov 2, 2023

I'm still seeing "risky cross-entry-point nested import" warnings:

That's fine in this case - it means that the other files will get inlined in the bundle, which we generally want to avoid - but in this case it's actually a good idea.

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.

I don't have projects to test it, but I assume you did, and the code looks very reasonable. Let's get this into the next alpha and wait for user feedback.

@alessbell alessbell merged commit 46ab032 into release-3.9 Nov 2, 2023
24 checks passed
@alessbell alessbell deleted the multipart-subscriptions-adapters branch November 2, 2023 13:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants