Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Allow data and errors in payload of SUBSCRIPTION_DATA #84

Merged
merged 4 commits into from Mar 3, 2017

Conversation

timsuchanek
Copy link
Contributor

Hi, at Graphcool, we have the use case, that sometimes we want to send data AND errors on the payload of SUBSCRIPTION_DATA at the same time.

This is especially needed for our permissions system, where it could be that the requesting client is only allowed to see some fields on a query. In that case we don't want to disable all other fields, so we send the allowed fields and an error that explains why some fields are null.

This PR just makes sure, that if there is the errors property, data is not just nulled, but is actually checked if there is data.

CHANGELOG.md Outdated
@@ -2,6 +2,9 @@

### vNEXT

### 0.5.4
Copy link
Contributor

Choose a reason for hiding this comment

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

These should just be placed under vNEXT as multiple different merge requests may get bundled into the next release.

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "subscriptions-transport-ws",
"version": "0.5.3",
"version": "0.5.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to change this in the MR.

src/client.ts Outdated
@@ -306,7 +306,8 @@ export class SubscriptionClient {
if (parsedMessage.payload.data && !parsedMessage.payload.errors) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably cleaner to conditionally set playloadData and playloadErrors and have only a single call to this.subscriptions[subId].handler.

@timsuchanek
Copy link
Contributor Author

Thanks for the feedback, good points, just updated the MR

@NeoPhi NeoPhi merged commit 39caa39 into apollographql:master Mar 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants