apollographql / apollo-rs

Spec compliant GraphQL Tools in Rust.
Apache License 2.0
576 stars 45 forks source link

feat(compiler): add schema coordinates #757

Closed goto-bus-stop closed 10 months ago

goto-bus-stop commented 11 months ago

Implements parsing and printing for the Schema Coordinates RFC.

Here, SchemaCoordinate is an enum of all the different kinds of coordinates. Each kind of coordinate is also its own type.

use apollo_compiler::name;
use apollo_compiler::coordinate::{SchemaCoordinate, FieldArgumentCoordinate};

let coord: SchemaCoordinate = "Type.field(argument:)".parse().unwrap();
assert_eq!(coord, SchemaCoordinate::FieldArgument(
    FieldArgumentCoordinate {
        ty: name!("Type"),
        field: name!("field"),
        argument: name!("argument"),
    },
));
use apollo_compiler::coord;

assert_eq!(coord!(@directive).to_string(), "@directive");
assert_eq!(coord!(@directive(arg:)).to_string(), "@directive(arg:)");
assert_eq!(coord!(Type).to_string(), "Type");
assert_eq!(coord!(Type.field).to_string(), "Type.field");
assert_eq!(coord!(Type.field(arg:)).to_string(), "Type.field(arg:)");

The goal of this API is:

Some other options:

goto-bus-stop commented 11 months ago

Oh hmm, a FieldCoordinate can also refer to an enum value. The RFC actually calls it a type attribute rather than a field: https://github.com/graphql/graphql-wg/blob/main/rfcs/SchemaCoordinates.md#typeattribute

So you can have Query.products referring to a field and ProductType.book referring to an enum value. that makes the typed lookup by coordinate idea a bit less interesting.

goto-bus-stop commented 11 months ago

I think for pointing out a specific implements, or pointing out a directive application, we would use both a coordinate + a string (or another coordinate) for the more specific thing in the error message. It does mean that the coordinates for error locations would often be a parent node of where the error actually occurred, but it's not gonna be super far away...

For example, if you do something like

type Example {
  name @deprecated(reason: false)
}

you could get an error with its location at coordinate Example.name, and the message formatting using two coordinates:

Expected type String, but Boolean was provided to `@deprecated(reason:)` on `Example.name`

(would need some copy workshopping to make it read well)

goto-bus-stop commented 11 months ago

Something else you can't point to is a schema definition, so we couldn't require a coordinate for diagnostics on those

e; As discussed in the call, the initial use for this would be for diagnostic messages, not really as a substitute for location information.

goto-bus-stop commented 10 months ago

As discussed in checkin, we keep lookup out for now, since we aren't really sure of the utility or about parts of the design (like the trait). Replaced coordinate: String uses in diagnostics with either a specific coordinate type or SchemaCoordinate.