kamilkisiela / apollo-angular

A fully-featured, production ready caching GraphQL client for Angular and every GraphQL server 🎁
https://apollo-angular.com
MIT License
1.5k stars 311 forks source link

After migration to v2 - Comma from query can be removed #1631

Closed dlarr closed 3 years ago

dlarr commented 3 years ago

Describe the bug

I am try to migrate from v1.10.0 to v2.2.0 I used the ng update apollo-angular schematics and things went pretty well.

Problem is with some request the resulting query within request payload removed sont comma that are mandatory for execution

Here is the request written in my code (some filters and field are parameters here) The interesting line is dateHistogram(period: "7d", metricAggregation: "max", timestampField: "timestamp") {

return gql(`
    query QualityHistoryRequest($project: String, $from: String) {
    dateHistogram(period: "7d", metricAggregation: "max", timestampField: "timestamp") {
      qualityAnalyses(
        filter: {
          op: AND
          criteria: [
            ${getFilters(context)}
            { field: "timestamp", comparator: GREATER_THAN, value: $from }
          ]
        }
      ) {
        timestamp
        measures {
           ${getMeasure(widgetConfig)}
        }
      }
    }
  }

The problem is that when the HTTP request is actually posted the commas are removed and replaced by a carriage return

image

In a copy/paste here is the result:

"query QualityHistoryRequest($project: String, $from: String) {
  dateHistogram(
    period: "7d"
    metricAggregation: "max"
    timestampField: "timestamp"
  ) {
    qualityAnalyses(
      filter: {op: AND, criteria: [{field: "projectId", comparator: MATCH, value: "something(:.*)?"}, {field: "branch", comparator: MATCH, value: "develop(/.*)?"}, {field: "timestamp", comparator: GREATER_THAN, value: $from}]}
    ) {
      timestamp
      measures {
        critical_violations
        blocker_violations
        major_violations
        minor_violations
        info_violations
        __typename
      }
      __typename
    }
    __typename
  }
}
"

See how commas have been removed ?

To Reproduce Steps to reproduce the behavior: I don't know

Expected behavior

These commas need to be kept

Environment:

Additional context

The problem may come from Apollo/Client but I can't find an answer talking about these commas replacement

dlarr commented 3 years ago

waw it's kinda dead around here :(

fetis commented 3 years ago

we're not dead, but slow :)

kamilkisiela commented 3 years ago

https://github.com/kamilkisiela/apollo-angular/pull/1642/commits/f6902f24082f4100baedafcdea01e703c7025522 it's totally fine and correct when it comes to graphql spec. We introduced operationPrinter option to write your own printing logic or even use stripIgnoredCharacters of graphql.