-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
The ability to represent empty objects #568
Comments
I think one interesting aspect of this is that you would still be able to query fragment Something on WeakPassword {
__typename
} Side note having empty types is also useful if you want to declare a type for the sole purpose of using |
The problem is it doesn't allow you define a schema with an empty value. Currently the workaround is to do the following: type A {
_: Boolean
} |
Type extension with type Query
extend type Query {
a: Boolean;
} |
Yes - I'm agreeing with you! |
What does it take to fix this issue? I found that simply commenting out the check, everything just works. (I'm using Apollo on the client and |
It's a spec change so somebody should champion it and move through all of the stages: |
For background, the requirement to have at least one field was introduced to the spec by 0599414. This change references graphql/graphql-js#368 where this was previously discussed (and then rejected). The main argument for rejection in that issue seems to me to have been:
In my opinion:
Allowing empty object/input object types has concrete use cases:
I'd be happy to submit a PR to simply remove this validation from the spec, as a strawman or proposal (edit: see #606). |
Supporting empty types, e.g. objects, input objects and interfaces without any fields, has concrete use cases. - Support for GraphQL APIs without any `Query` operation type fields, for example, if the API only support mutations or subscriptions (see graphql#490). This allows defining an empty `Query` object type, while still supporting introspection. - Support for algebraic data types (see graphql#568), where `__typename` is the only relevant field - Potentially to support "well-known" (but empty) extensions of the introspection schema (see graphql#300) This is a minimalist spec change, which simply removes the relevant items under the type validation sub sections. It would probably be helpful to motivate the change and mention that one or more fields _should_ be present for a (typically) useful schema. The requirement for composite types (object, input objects and interfaces) to define "one or more fields" was introduced in 0599414. This change references graphql/graphql-js#368, motivating the change with: > Since GraphQL always requires you to select fields down to scalar values, an Object type without any defined fields cannot be accessed in any way in a query. > This could be even more problematic for Input Objects where a required input object argument with no fields could result in a field that is impossible to query without producing an error. With regards to these objections: - It's always possible to select `__typename` and therefore even empty object types can be useful (as e.g. algebraic data types) - Passing an empty input object appears syntactically valid: ```gql mutation { update(input: {}) { name } } ``` I think this proposal fulfills the guiding principles by enabling new capabilities motivated by real use cases. This change does not make any previously valid schema invalid, so largely preseves backwards compatibility. Fixes graphql#568 and graphql#490 at the specification level.
Supporting empty types, e.g. objects, input objects and interfaces without any fields, has concrete use cases. - Support for GraphQL APIs without any `Query` operation type fields, for example, if the API only support mutations or subscriptions (see graphql#490). This allows defining an empty `Query` object type, while still supporting introspection. - Support for algebraic data types (see graphql#568), where `__typename` is the only relevant field - Potentially to support "well-known" (but empty) extensions of the introspection schema (see graphql#300) This is a minimalist spec change, which simply removes the relevant items under the type validation sub sections. It would probably be helpful to motivate the change and mention that one or more fields _should_ be present for a (typically) useful schema. The requirement for composite types (object, input objects and interfaces) to define "one or more fields" was introduced in 0599414. This change references graphql/graphql-js#368, motivating the change with: > Since GraphQL always requires you to select fields down to scalar values, an Object type without any defined fields cannot be accessed in any way in a query. > This could be even more problematic for Input Objects where a required input object argument with no fields could result in a field that is impossible to query without producing an error. With regards to these objections: - It's always possible to select `__typename` and therefore even empty object types can be useful (as e.g. algebraic data types) - Passing an empty input object appears syntactically valid: ```gql mutation { update(input: {}) { name } } ``` I think this proposal fulfills the guiding principles by enabling new capabilities motivated by real use cases. This change does not make any previously valid schema invalid, so largely preseves backwards compatibility. Fixes graphql#568 and graphql#490 at the specification level.
These restrictions also apply to interfaces, this makes it really difficult to represent domain objects in OOP that leverage interfaces. It's pretty common to have interfaces with no properties defined. |
@vanga for this you actually the right thing to do is to use union types. Interfaces with no properties are used in oop as marker interfaces due to the lack of unions. With unions interfaces with no fields are not necessary. |
Hi @michaelstaib, Thanks for the suggestion. But, I am not sure if it solves the real issue here. Interface represents a hierarchical relation between entities where as union type just a wrapper type for a group of entities (sibling and independent entities?) IMHO. This hierarchical relationship can be of any length. And, one needn't have fields specified at all levels of hierarchy at a given point in time depending on how one chooses to represent their entities. Unfortunately this is the kind of data model which we have to represent using graphql. And, I do think that it's not weird to have such data models in practice. Ex: I understand that graphql doesn't yet have support for one interface inheriting from another (but there is a feature request for this IIRC which is WIP). We solve this a bit differently by having one type inherit from multiple types. B inherits A I can have a schema such a way that a query on A will consider data from all the children B, C, D by default without having to explicitly specify fragments, because consumers don't have to know which/what all children exist, which is the point of having an interface. |
@vanga the interface inheriting interface is not meant to introduce those hierarchies but make validating fragment selections possible. If you read through this you will notice that on each level the interface has to be implemented. If you still think that this should be introduced I would suggest opening another issue and describing your use case. Each change should be caused by a distinct need for a use-case and described in its own issue. |
This makes sense to me too. Since errors are already injected in the response, an empty payload object can be a simple indicator of success. It's less confusing and wasteful than adding the workaround boolean field. |
GraphQL specification requires Query type to be present as it is required to run introspection query. Per specification it also shouldn't be possible to generate empty complex types (objects, input objects or interfaces) and they should expose at least a single field. Since root Query type is a special GraphQLObjectType it also has to expose at least a single field. Breaking changes: * at least single Query is required when generating the schema * split `didGenerateQueryType` hook (and corresponding `Mutation` and `Subscription` hooks) into `didGenerateQueryFieldType` (previous functionality) and `didGenerateQueryObjectType` hooks to allow more granular control when generating the schema * default `SchemaGeneratorHooks` now performs validation of generated object types (including special query, mutation and subscription types), input object types and interfaces to ensure we don't generate empty complex object types see: graphql/graphql-spec#490 and graphql/graphql-spec#568
) * BREAKING CHANGE: empty complex types should fail schema generation GraphQL specification requires Query type to be present as it is required to run introspection query. Per specification it also shouldn't be possible to generate empty complex types (objects, input objects or interfaces) and they should expose at least a single field. Since root Query type is a special GraphQLObjectType it also has to expose at least a single field. Breaking changes: * at least single Query is required when generating the schema * split `didGenerateQueryType` hook (and corresponding `Mutation` and `Subscription` hooks) into `didGenerateQueryFieldType` (previous functionality) and `didGenerateQueryObjectType` hooks to allow more granular control when generating the schema * default `SchemaGeneratorHooks` now performs validation of generated object types (including special query, mutation and subscription types), input object types and interfaces to ensure we don't generate empty complex object types see: graphql/graphql-spec#490 and graphql/graphql-spec#568
I am currently using union types for modeling a process with multiple steps. I also hit the limitation that sometimes I do not need any field on an object type. I wanna share the use-case so it can be considered as an example for a possible RFC. type Result {
some: String!
otherValue: Boolean!
}
type Error {
code: String!
message: String!
}
type AnalysisNotStarted {
# noop field
_: Boolean
}
type AnalysisEnqueued {
# noop field
_: Boolean
}
type AnalysisProcessing {
# noop field
_: Boolean
}
type AnalysisFinished {
result: Result!
}
type AnalysisFailed {
error: Error!
}
union Analysis = AnalysisNotStarted | AnalysisEnqueued | AnalysisProcessing | AnalysisFinished | AnalysisFailed
type Record {
id: ID!
analysis: Analysis!
} |
I cannot understand why you don't simply use an While I can understand it makes sense to have empty "tagging" types in general, in this particular place it looks like a misuse to me. |
@jdehaan I don't want a nullable field. When leveraging type generation on the graphql api consumer e.g. by using relay or apollo-client (with apollo-codegen) you end up having to do an enum check + a null check. I think a union is the "cleaner" and more type-safe/"state-machine"-like approach. This becomes "weirder" when having multiple analysis types: type Error {
message: String!
}
type Analysis1Result {
foo: String!
}
type Analysis2Result {
bar: String!
}
union Analysis1ErrorOrResult = Error | Analysis1Result
union Analysis2ErrorOrResult = Error | Analysis2Result
enum AnalysisStatus { not_started enqueued processing finished failed }
type Record {
id: ID!
analysis1Status: AnalysisStatus!
analysis1ErrorOrResult: Analysis1ErrorOrResult
analysis2Status: AnalysisStatus!
analysis2ErrorOrResult: Analysis1ErrorOrResult
} vs union Analysis1 = ... # see above (https://github.com/graphql/graphql-spec/issues/568#issuecomment-630633807)
union Analysis2 = ... # see above (https://github.com/graphql/graphql-spec/issues/568#issuecomment-630633807)
type Record {
id: ID!
analysis1: Analysis1!
analysis2: Analysis2!
} Also, it might be possible that additional metadata such as an enqueuedDate or startedProcessingDate on the |
Thanks for the elaboration. I can understand better now how it can make sense. |
@jdehaan We are actually doing the enum + nullable field approach right now and are realizing that it limits us in the future extensibility of the schema. While figuring out how to do better I found this issue and wanted to provide my input :) |
@n1ru4l Since there is no structural difference between your "ongoing" analysis types, I'd argue that the following is how you can structure your schema. type Result {
some: String!
otherValue: Boolean!
}
type Error {
code: String!
message: String!
}
enum AnalysisOngoingStatus {
NOT_STARTED
ENQUEUED
PROCESSING
}
type AnalysisOngoing {
status: AnalysisOngoingStatus!
}
type AnlaysisFinished {
result: Result!
}
type AnalysisFailed {
error: Error!
}
union Analysis = AnalysisOngoing | AnlaysisFinished | AnalysisFailed
type Record {
id: ID!
analysis: Analysis!
} Basically you only create new types when you've actually got different fields to select. I think this is a good schema in itself – if this were a final schema, all your "ongoing analysis" types are just a single type because they all have a single, common field. The main drawback of this is, in my opinion, extensibility and schema evolution. If you've already defined (possibly empty) types, it's easier to add more fields to them than to introduce a new type. I personally think this is an interesting argument, but it could lead to very dubious "best practices" evolving where everything should be its own type. FWIW, I was the author behind a PR that would have allowed this (#606). I closed it in response to Working Group feedback but there may be reasons to pick it up again. |
These "empty types" are espically important in languages that support the concept of pattern matching and sum types/disjoint uions/discriminated unions/variants/tagged unions/whatever you want to call it. These are a features in scala, haskell, rust, swift, typescript, flow, modern c++(std::variant), and most modern and functional languages. These "empty" values as you describe them aren't actually empty. They can repersent completelty different states of the system. You don't need only one "empty" type, because each empty type is discriminated on its C-style enums combined with nullable properties don't cut it in terms of ergonomics or type safety. It seems like a big oversight that graphql supports tagged union types but doesn’t seem to understand not all members of the union need to have additional associated data to be meaningful. |
The funniest thing is that some GraphQL implementations have this "check" in place, and some don't (since this is not technical limitation, but rather an arbitrary validity rule). |
The use case I have for this is modularizing my code. In different modules, I have |
I have a similar issue but instead of an arbitrary type i just did
|
Two simple use cases:
The success type in this use case is sufficient to indicate success, and needs no additional fields. Yet GQL forces me to define something in there, even if it is useless and confusing.
One approach could be:
There is no point to adding any kind of data field to
There is a lot of pushback in the rejected RFC #606 to representing ADTs as errors, but neither of these use cases have anything to do with errors. Unless I'm missing some idiomatic approach to solving these simple problems, this is a bit silly. |
Mutations should (arguably) return the mutated object. But the Enum-based solution which was proposed in #568 (comment) is definitely not a great thing to implement, as it undermines the ability to add fields to those events which were merged into enum. Removing enum fields can be dangerous, especially if the client has some sort of local DB (very common for mobile dev). |
…xpediaGroup#541) * BREAKING CHANGE: empty complex types should fail schema generation GraphQL specification requires Query type to be present as it is required to run introspection query. Per specification it also shouldn't be possible to generate empty complex types (objects, input objects or interfaces) and they should expose at least a single field. Since root Query type is a special GraphQLObjectType it also has to expose at least a single field. Breaking changes: * at least single Query is required when generating the schema * split `didGenerateQueryType` hook (and corresponding `Mutation` and `Subscription` hooks) into `didGenerateQueryFieldType` (previous functionality) and `didGenerateQueryObjectType` hooks to allow more granular control when generating the schema * default `SchemaGeneratorHooks` now performs validation of generated object types (including special query, mutation and subscription types), input object types and interfaces to ensure we don't generate empty complex object types see: graphql/graphql-spec#490 and graphql/graphql-spec#568
Completely agree with everything people have been saying here. I noticed one comment in the corresponding PR that was never explicitly addressed, and I want to comment on it here:
One difference is you can nest Also, to add on to an earlier comment:
This becomes even more apparent the more fields you have # desired
type Foo = A | B
type A {}
type B { x: Int!; y: Int!; z: Int! }
# workaround
enum FooType = A | B
type Foo {
type: FooType!
x: Int
y: Int
z: Int
} // desired
switch (data.__typename) {
case "A": return 0
case "B": return data.x + data.y + data.z
}
// workaround
switch (data.type) {
case FooType.A: return 0
case FooType.B: {
const { x, y, z } = data
if (x === null || y === null || z === null) {
throw new Error("Should not happen")
}
return x + y + z
}
} |
Currently content items for non-withdrawn editions will include an empty object for the `withdrawn_notice`. We opted to return `nil` instead, which feels more conventional (for APIs in general) and is also the [recommended way of doing things in GraphQL][1]. Implementing an empty object in GraphQL requires some fairly [hacky code][2]. We'll need to ensure that consumers can handle this field being `nil` before any move to use the GraphQL endpoint in production [1]: https://spec.graphql.org/draft/#sel-HAHZhCFBABPBT5pS [2]: graphql/graphql-spec#568 (comment) Co-authored-by: George <george@dxw.com>
Currently content items for non-withdrawn editions will include an empty object for the `withdrawn_notice`. We opted to return `nil` instead, which feels more conventional (for APIs in general) and is also the [recommended way of doing things in GraphQL][1]. Implementing an empty object in GraphQL requires some fairly [hacky code][2]. We'll need to ensure that consumers can handle this field being `nil` before any move to use the GraphQL endpoint in production [1]: https://spec.graphql.org/draft/#sel-HAHZhCFBABPBT5pS [2]: graphql/graphql-spec#568 (comment) Co-authored-by: George <george@dxw.com>
Currently content items for non-withdrawn editions will include an empty object for the `withdrawn_notice`. We opted to return `nil` instead, which feels more conventional (for APIs in general) and is also the [recommended way of doing things in GraphQL][1]. Implementing an empty object in GraphQL requires some fairly [hacky code][2]. We'll need to ensure that consumers can handle this field being `nil` before any move to use the GraphQL endpoint in production [1]: https://spec.graphql.org/draft/#sel-HAHZhCFBABPBT5pS [2]: graphql/graphql-spec#568 (comment) Co-authored-by: George <george@dxw.com>
Currently content items for non-withdrawn editions will include an empty object for the `withdrawn_notice`. We opted to return `nil` instead, which feels more conventional (for APIs in general) and is also the [recommended way of doing things in GraphQL][1]. Implementing an empty object in GraphQL requires some fairly [hacky code][2]. We'll need to ensure that consumers can handle this field being `nil` before any move to use the GraphQL endpoint in production [1]: https://spec.graphql.org/draft/#sel-HAHZhCFBABPBT5pS [2]: graphql/graphql-spec#568 (comment) Co-authored-by: George <george@dxw.com>
Currently content items for non-withdrawn editions will include an empty object for the `withdrawn_notice`. We opted to return `nil` instead, which feels more conventional (for APIs in general) and is also the [recommended way of doing things in GraphQL][1]. Implementing an empty object in GraphQL requires some fairly [hacky code][2]. We'll need to ensure that consumers can handle this field being `nil` before any move to use the GraphQL endpoint in production [1]: https://spec.graphql.org/draft/#sel-HAHZhCFBABPBT5pS [2]: graphql/graphql-spec#568 (comment) Co-authored-by: George <george@dxw.com>
Based on this: graphql/graphql-js#937 (comment)
Currently we can't represent an empty object type:
This fails with
graphql-js
with the following error:The usecase is legit though.
There is an explicit check to disallow types with zero fields at
graphql-js
:https://github.com/graphql/graphql-js/blob/4116e2fc4fe36688f683258388f4a2d52076d199/src/type/validate.js#L273-L278
How about explicitly allowing empty fields in the spec? It's super useful for implementing algebraic types.
The text was updated successfully, but these errors were encountered: