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

Release 3.0 #250

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Release 3.0 #250

wants to merge 6 commits into from

Conversation

hwillson
Copy link
Member

This release PR will serve to collect significant new features, deprecation warnings, and minor breaking changes that we intend to release in [email protected]. It should not be merged until we're ready to release 3.0.

Initial CHANGELOG placeholder for release 3.0.
* Add a(n optional) generic type map to PubSub.

The class PubSub now has a generic type parameter so its methods `publish` and `subscribe` can be **optionally** type-checked by TypeScript.

* Added part for PubSub generic.

* Slight README tweaks to align formatting / adjust grammar a bit

* Changelog update

Co-authored-by: hwillson <[email protected]>
@hwillson hwillson added this to the Release 3.0 milestone Nov 25, 2021
n1ru4l and others added 4 commits November 25, 2021 12:52
…ypes (#232)

* fix(typings): return AsyncIterableIterator instead of AsyncIterator

BREAKING fixes the type annotation of the abstract class PubSubEngine. According to the TypeScript type-defintion a `PubSubAsyncIterator` instance is actually a `AsyncIterableIterator` instead of an  `AsyncIterator`. The typing of `PubSubAsyncIterator` is way more convenient as it allows iterating over it with the `for await (const foo of iterator) { doSth() }` syntax, which is super handy for filtering or mapping (See https://gist.github.com/n1ru4l/127178705cc0942cad0e45d425e2eb63 for some example operators).

* remove iterall

* rename asyncIterator method to asyncIterableIterator.

* Slight tweaks based on graphql-js 16 changes

* Changelog update

Co-authored-by: hwillson <[email protected]>
* Failing case: pubsub asyncIterator should accept a readonly string[]

* fix: Allow readonly triggers to be passed to PubSubEngine.asyncIterator

* Changelog update

Co-authored-by: hwillson <[email protected]>
* Allow resolver fn to be async

* Changelog updates

Co-authored-by: hwillson <[email protected]>
@maxpain
Copy link

maxpain commented Jan 16, 2022

Any updates?

@Narretz
Copy link

Narretz commented Jan 28, 2022

Are there any more breaking changes planned? If not, could the release happen? Or even if, could the release happen? There's always the next major version for more breaking changes.

@tubbo
Copy link

tubbo commented Feb 11, 2022

@hwillson hey this has been blocking a major version upgrade of graphql codegen for us, any chance you could release soon?

tim-stasse added a commit to rhdemo/2022-game-app that referenced this pull request Feb 22, 2022
Patch graphql-subscriptions with changes from the [release 3.0 PR](apollographql/graphql-subscriptions#250) until it's merged
@daphnesmit
Copy link

Same 👯

@Hetch3t
Copy link

Hetch3t commented May 28, 2022

Any new improvements planned? What's holding you from merging it finally?

@maxpain
Copy link

maxpain commented Jun 20, 2022

Any news?

@Ryiski
Copy link

Ryiski commented Nov 27, 2022

1 year and still not ready? not to be rude or anything, what's the hold-up

@aidenfoxx
Copy link

Any update would be appreciated? 🙏

@MrDesjardins
Copy link

Hello,

I am reading the latest (V4) Apollo documentation that recommend using the graphql-subscriptions library with subscription. However, the type, as mentioned by many people in this thread, are not compatible. I believe this pull request will solve this issue. Apollo is quite a popular and the are fading out their v3 for v4.

There is a discussion in the graphql-codegen that generate code to use with Apollo that highlight that the issue is not on the generated type. Someone in the Codegen discussion gives a quick hack to fix the issue:

subscribe: () => ({
        [Symbol.asyncIterator]: () => pubsub.asyncIterator('NUMBER_INCREMENTED'),
      }),

However, the hack is only for type. Doing:

subscribe: () => pubsub.asyncIterator("NUMBER_INCREMENTED") as any,

Also fix the problem.

The first comment of this thread is from Nov 25, 2021 and we are now almost over with 2022. The direction of this library is unclear to me but it is an helpful one and it would be great to see it moving forward to continue to work with TypeScript without hacks. Have a great day.

export type FilterFn<TSource = any, TArgs = any, TContext = any> = (rootValue?: TSource, args?: TArgs, context?: TContext, info?: any) => boolean | Promise<boolean>;
export type ResolverFn<TSource = any, TArgs = any, TContext = any> = (rootValue?: TSource, args?: TArgs, context?: TContext, info?: any) => AsyncIterator<any>;
export type ResolverFn<TSource = any, TArgs = any, TContext = any> = (rootValue?: TSource, args?: TArgs, context?: TContext, info?: any) => AsyncIterator<any> | Promise<AsyncIterator<any>>;
Copy link

Choose a reason for hiding this comment

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

I think to have proper typing there needs to be a distinction between the asyncIteratorFn received as input which needs to return an AsyncIterator and the withFiltermethod that actually returns a AsyncIterableIterator

Should split ResolverFn in 2 types to make the distinction.

@Aeolun
Copy link

Aeolun commented Aug 10, 2023

@hwillson Is this something that is still on the agenda? Migrating to apollo v4 would be a lot more pleasant without conflcting types.

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