Make GraphQL subscriptions fire a redux action #775

helfer · 2016-10-15T00:36:42Z

TODO:

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
Add your name and email to the AUTHORS file (optional)
If this was a change that affects the external API, update the docs and post a link to the PR in the discussion

stubailo

LGTM, some responses to code comments

stubailo · 2016-10-15T08:09:50Z

src/core/QueryManager.ts

+              subscriptionId: subId,
+              extraReducers: this.getExtraReducers(),
+            });
+            // It's slightly awkward that the data for subscriptions doesn't come from the store.


I think this is more or less reasonable since subscriptions usually don't represent an updating data source. Or rather, whether or not it comes from the store depends on whether you are using a subscription in place of a polling query, or as an event source.

stubailo · 2016-10-15T08:10:49Z

src/data/store.ts

+      const clonedState = assign({}, previousState) as NormalizedCache;
+
+      // TODO REFACTOR: is writeResultToStore a good name for something that doesn't actually
+      // write to "the" store?


What do you mean, "it doesn't write to the store?"

helfer added 3 commits October 15, 2016 01:07

make subscriptions fire redux action

636c186

update CHANGELOG.md

6be9b13

add a test

9f0a06f

stubailo force-pushed the subscriptions-api branch from fb8e936 to 9f0a06f Compare October 15, 2016 08:08

stubailo reviewed Oct 15, 2016

View reviewed changes

stubailo merged commit d27305a into master Oct 15, 2016

stubailo deleted the subscriptions-api branch October 15, 2016 22:05

github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make GraphQL subscriptions fire a redux action #775

Make GraphQL subscriptions fire a redux action #775

helfer commented Oct 15, 2016 •

edited

Loading

stubailo left a comment

stubailo Oct 15, 2016

stubailo Oct 15, 2016

Make GraphQL subscriptions fire a redux action #775

Make GraphQL subscriptions fire a redux action #775

Conversation

helfer commented Oct 15, 2016 • edited Loading

stubailo left a comment

Choose a reason for hiding this comment

stubailo Oct 15, 2016

Choose a reason for hiding this comment

stubailo Oct 15, 2016

Choose a reason for hiding this comment

helfer commented Oct 15, 2016 •

edited

Loading