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.

Sign up for GitHub

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

Jump to bottom

Add subscriptions support to makeRemoteExectuableSchema #563

Merged
merged 7 commits into from
Jan 3, 2018
Merged

Add subscriptions support to makeRemoteExectuableSchema #563

merged 7 commits into from
Jan 3, 2018

Conversation

schickling
Copy link
Contributor

@schickling schickling commented Jan 2, 2018 •
edited
Loading

  • If this PR is a new feature, 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
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass
  • Update CHANGELOG.md with your change

@schickling schickling changed the title WIP: Added subscriptions support to makeRemoteExectuableSchema Add subscriptions support to makeRemoteExectuableSchema Jan 2, 2018
abhiaiyer91
abhiaiyer91 reviewed Jan 3, 2018
View reviewed changes
src/stitching/makeRemoteExecutableSchema.ts Outdated
};
const observable = execute(link, operation);

const pubSub = new PubSub();
Copy link

Choose a reason for hiding this comment

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

I think users of this should supply their own pubsub right?

Copy link

Choose a reason for hiding this comment

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

Though i understand the usage here

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 on providing pubsub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any API design suggestions for this?

@abhiaiyer91
Copy link

This is awesome.

@freiksenet
Copy link
Contributor

Looks good so far. We need tests for makeRemoteExecutableSchema, at least for subscriptions.

@schickling
Copy link
Contributor Author

Btw, the tests work. It's just Travis failing. I recommend switching over to CircleCI.

@freiksenet freiksenet merged commit 342bece into ardatan:master Jan 3, 2018
@freiksenet freiksenet deleted the remote-subscriptions branch January 3, 2018 13:21
@stubailo
Copy link
Contributor

stubailo commented Jan 3, 2018

CircleCI has its own issues - I think we’ll stick with Travis for now!

freiksenet added a commit that referenced this pull request Feb 15, 2018
@freiksenet
Next api
08e2058 
Squashed commit of the following:

commit a108877910e0556759f92e9acab4643c63be13ff
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Wed Jan 31 16:05:00 2018 +0200

    First transforms

commit 334802c
Merge: 99884b6 e9c0eb5
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Fri Jan 26 12:27:57 2018 +0200

    Merge remote-tracking branch 'origin/master' into next-api

commit 99884b6
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Fri Jan 26 11:57:15 2018 +0200

    Restored valid fragments

commit f1b2432
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Thu Jan 25 14:55:57 2018 +0200

    Progress :P

commit db2806a
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Thu Jan 25 13:35:35 2018 +0200

    Fragment replacement

commit e9c0eb5
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Wed Jan 24 18:28:12 2018 +0200

    v2.19.0 (#594)

commit e33c215
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Wed Jan 24 16:31:37 2018 +0200

    Progress

commit 464670f
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Tue Jan 23 15:03:40 2018 +0200

    More progress

commit 22d2295
Author: Sebastian Richter <sebastian.max.richter@gmail.com>
Date:   Tue Jan 23 12:45:49 2018 +0100

    Also recreate astNode for fields (#580)

    * Also recreate astNode for fields

    In a [previous commit](fd9f626) we added the `astNode` property in the `reacreateCompositeType` function. That resulted in cache control working with schema stitching but only for GraphQL Types. By recreating the `astNode` prop also in `fieldToFieldConfig` cache control also works for fields. This is required for caching fields and hence queries.

    * Add ast to input field node too

commit 03ad432
Author: Renato Benkendorf <renatobenks@users.noreply.github.com>
Date:   Tue Jan 23 08:35:00 2018 -0200

    Fix resolvers to accept and move forward args with zero or false values (#586)

    Fixing the args with zero value or false

commit 72ac16f
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Tue Jan 23 11:37:02 2018 +0200

    Upgrade remap-istanbul (#590)

commit 5420557
Merge: 24a0f3f 3ef557a
Author: Sashko Stubailo <sashko@meteor.com>
Date:   Mon Jan 22 11:55:07 2018 -0800

    Merge pull request #584 from shackpank/double_deps

    Remove graphql-subscriptions from devDependencies

commit 3ef557a
Author: Ollie Buck <git@olliebuck.com>
Date:   Fri Jan 19 14:40:03 2018 +0000

    Remove graphql-subscriptions from devDependencies

commit 7e4efe9
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Thu Jan 18 16:38:13 2018 +0200

    Progress

commit 24a0f3f
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Wed Jan 10 12:36:43 2018 +0200

    v2.18.0 (#574)

commit 966e102
Author: Amit-A <Amit-A@users.noreply.github.com>
Date:   Wed Jan 10 02:53:56 2018 -0500

    Fixing broken links (#573)

commit 94b8f68
Author: Pascal Kaufmann <pozylon@gmail.com>
Date:   Tue Jan 9 12:51:53 2018 +0100

    Fix fragment filtering edge case when a type implements multiple interfaces (#546)

    * add failing test for multi interface issue

    * interface / interface always implements an abstract type

    * changelog

    * the linter always should be happy

    * Update CHANGELOG.md

commit 608414b
Author: Tim Griesser <tgriesser10@gmail.com>
Date:   Tue Jan 9 06:15:12 2018 -0500

    Allow IEnumResolver values to be number type (#568)

    * Allow IEnumResolver values to be number type

commit 6177034
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Tue Jan 9 11:26:04 2018 +0200

    v2.17.0 (#571)

commit fd9f626
Author: Sebastian Richter <sebastian.max.richter@gmail.com>
Date:   Tue Jan 9 09:25:16 2018 +0100

    Include astNode in schema recreation (#569)

    * Update schemaRecreation.ts

    `mergeSchemas` does not set `astNode` properties, when recreating types.
    But `CacheControlExtension.willResolveField` uses the `astNode` property in order to get the `cacheControl` directives.

commit 4f489d3
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Wed Jan 3 15:40:21 2018 +0200

    v2.16.0 (#564)

commit c4c5d91
Author: VimalRaj Selvam <email2vimalraj@gmail.com>
Date:   Wed Jan 3 19:00:16 2018 +0530

    #544 - Update docs and tests to reflect new strings-as-descriptions pattern in new GraphQL SDL (#559)

    * #64 - Update docs and tests to reflect new strings-as-descriptions pattern in new GraphQL SDL

    * Make tests compatible with graphql v0.11

    * Fix test errors

    * Remove extra space

    * Fix review comments

commit 66d58bb
Author: Alvis Tang <alvis@users.noreply.github.com>
Date:   Wed Jan 3 13:25:59 2018 +0000

    fix: correct the dependency issue in typescript caused by #445 (#561)

    fix #472

commit 342bece
Author: Johannes Schickling <schickling.j@gmail.com>
Date:   Wed Jan 3 14:21:35 2018 +0100

    Add subscriptions support to `makeRemoteExectuableSchema` (#563)

    Added subscriptions support to makeRemoteExectuableSchema

commit 8e8e348
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Tue Jan 2 14:50:06 2018 +0200

    v2.15.0 (#562)

commit f2905ac
Author: Johannes Schickling <schickling.j@gmail.com>
Date:   Tue Jan 2 13:40:56 2018 +0100

    Add document validation to `delegateToSchema` (#551)

    * Add document validation to `delegateToSchema`

commit 59592e1
Merge: 3e24c69 3612216
Author: Sashko Stubailo <sashko@meteor.com>
Date:   Thu Dec 28 10:02:28 2017 -0800

    Merge pull request #556 from enaqx/patch-4

    doc: fix Generating schema URL on Mocking page

commit 3612216
Author: Sashko Stubailo <sashko@meteor.com>
Date:   Thu Dec 28 10:00:16 2017 -0800

    Update mocking.md

commit 3e24c69
Merge: 7089bbf 42d180d
Author: Sashko Stubailo <sashko@meteor.com>
Date:   Thu Dec 28 09:58:31 2017 -0800

    Merge pull request #557 from tbroadley/fix-typos

    docs: fix typos

commit 42d180d
Merge: 5130cf6 7089bbf
Author: Sashko Stubailo <sashko@meteor.com>
Date:   Thu Dec 28 09:57:11 2017 -0800

    Merge branch 'master' into fix-typos

commit 7089bbf
Merge: 3a58286 5048434
Author: Sashko Stubailo <sashko@meteor.com>
Date:   Thu Dec 28 09:52:44 2017 -0800

    Merge pull request #552 from mklopets/patch-2

    Fix link to Apollo Link docs

commit 5048434
Author: Marko Klopets <mklopets@users.noreply.github.com>
Date:   Thu Dec 28 14:23:07 2017 +0200

    Re-fix link

commit 5130cf6
Author: Thomas Broadley <buriedunderbooks@hotmail.com>
Date:   Mon Dec 25 17:45:44 2017 -0500

    docs: fix typos

commit 80f2f4c
Author: Nick Raienko <enaqxx@gmail.com>
Date:   Mon Dec 25 09:19:03 2017 +0200

    doc: fix Generating schema URL on Mocking page

commit e295019
Author: Marko Klopets <mklopets@users.noreply.github.com>
Date:   Thu Dec 21 13:18:59 2017 +0200

    Fix link to Apollo Link docs

commit 3a58286
Merge: 259f22e 971e96e
Author: Sashko Stubailo <sashko@meteor.com>
Date:   Wed Dec 20 11:26:55 2017 -0800

    Merge pull request #548 from apollographql/create-2.14.1

    v2.14.1

commit 971e96e
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Wed Dec 20 12:44:14 2017 +0200

    v2.14.1

commit 259f22e
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Wed Dec 20 12:42:16 2017 +0200

    Guard against schemas without QueryType (#547)

commit 1e3bb14
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Tue Dec 19 14:06:13 2017 +0200

    Check null resolvers

commit 2e2c9b1
Author: Mikhail Novikov <freiksenet@gmail.com>
Date:   Mon Dec 11 16:09:06 2017 +0200

    Simplify API
@dependencies dependencies bot mentioned this pull request Mar 19, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@freiksenet freiksenet freiksenet left review comments

@abhiaiyer91 abhiaiyer91 abhiaiyer91 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@schickling @abhiaiyer91 @freiksenet @stubailo