strawberry icon indicating copy to clipboard operation
strawberry copied to clipboard

(Relay) Output ID instead of GlobalID in GraphQL schema

Open aryaniyaps opened this issue 1 year ago β€’ 5 comments

Request to output the ID scalar instead of GlobalID, while generating the GraphQL schema

My understanding is that GlobalID was meant to be an internal helper that resolves nodes, but ended up being a custom scalar.

Feature Request Type

  • [ ] Core functionality
  • [x] Alteration (enhancement/optimization) of existing feature(s)
  • [ ] New behavior

Description

Currently, while using the relay integration, and working with node types, like the example code below:

import strawberry
from strawberry import relay


@strawberry.type
class Fruit(relay.Node):
    code: relay.NodeID[int]
    name: str
    weight: float

    @classmethod
    def resolve_nodes(
        cls,
        *,
        info: strawberry.Info,
        node_ids: Iterable[str],
        required: bool = False,
    ):
        return [
            all_fruits[int(nid)] if required else all_fruits.get(nid)
            for nid in node_ids
        ]

We get a schema output like this:

scalar GlobalID

interface Node {
  id: GlobalID!
}

type Fruit implements Node {
  id: GlobalID!
  name: String!
  weight: Float!
}

But in the relay specification, which the GlobalID scalar is based on: https://relay.dev/graphql/objectidentification.htm

calls this scalar by the name ID, and not GlobalID.

I think that there is no mention of a custom scalar to be returned for object identification.

This leads to a lot of issues while working with client libraries such as relay, where directives expect the return type to be the scalar type of ID, and not GlobalID.

An example relay compiler error is shown below:

> [email protected] relay
> relay-compiler

[INFO] [default] compiling...
[ERROR] Error: βœ–οΈŽ Invalid use of @deleteEdge on field 'deletedTodoId'. Expected field type 'ID', got 'GlobalID'.

  client/src/components/home-page/Todo.tsx:20:21
   19 β”‚     deleteTodo(todoId: $todoId) {
   20 β”‚       deletedTodoId @deleteEdge(connections: $connections)
      β”‚                     ^^^^^^^^^^^
   21 β”‚     }

[ERROR] Compilation failed.
[ERROR] Unable to run relay compiler. Error details: 
Failed to build:
 - Validation errors: 1 error(s) encountered above.

Output GraphQL schema (After requested change)


interface Node {
 id: ID!
}

type Fruit implements Node {
 id: ID!
 name: String!
 weight: Float!
}

It would be nice if we could change the GlobalID scalar being generated to ID

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

aryaniyaps avatar Jun 28 '24 04:06 aryaniyaps

Here is a temporary solution for anyone who has the same issue, until this has been resolved:

import strawberry
from strawberry.relay import GlobalID

# temporary hack until strawberry fixes relay ID scalar generation
ID = strawberry.scalar(
    strawberry.ID,
    serialize=lambda value: str(value),
    parse_value=lambda value: GlobalID.from_id(value=value),
)

schema = Schema(
    query=query,
    mutation=mutation,
    scalar_overrides={GlobalID: ID},
)

aryaniyaps avatar Jun 28 '24 04:06 aryaniyaps

Thanks for creating an issue for this. There is already a discussion (#3177) and a PR (#3180) for this. Feel free to join the conversation over there as well.

DoctorJohn avatar Jul 04 '24 21:07 DoctorJohn

@aryaniyaps your temporary solution doesn't seem to work for me. I'm getting this: TypeError: Query fields cannot be resolved.

What version of strawberry are you using?

rodaan avatar Jul 05 '24 17:07 rodaan

@aryaniyaps your temporary solution doesn't seem to work for me. I'm getting this: TypeError: Query fields cannot be resolved.

What version of strawberry are you using?

I think the issue is, after creating a scalar like this;

ID = strawberry.scalar(
    strawberry.ID,
    serialize=lambda value: str(value),
    parse_value=lambda value: GlobalID.from_id(value=value),
)

You are using strawberry.ID in your schema. You should be using your own ID scalar instead.

I'm on version 0.235.0 btw

aryaniyaps avatar Jul 06 '24 12:07 aryaniyaps

Here is a temporary solution for anyone who has the same issue, until this has been resolved:

import strawberry
from strawberry.relay import GlobalID

# temporary hack until strawberry fixes relay ID scalar generation
ID = strawberry.scalar(
    strawberry.ID,
    serialize=lambda value: str(value),
    parse_value=lambda value: GlobalID.from_id(value=value),
)

schema = Schema(
    query=query,
    mutation=mutation,
    scalar_overrides={GlobalID: ID},
)

This was helpful, thank you

pfcodes avatar Jul 07 '24 04:07 pfcodes

The temporary fix using scalar_overrides does not work for projects that use both strawberry.ID and GlobalID.

import strawberry
from strawberry.relay.types import GlobalID

RelayID = strawberry.scalar(
    strawberry.ID,
    serialize=lambda value: str(value),
    parse_value=lambda value: GlobalID.from_id(value=value),
)


@strawberry.type
class Query:
    @strawberry.field
    def hello(self, id: GlobalID) -> str:
        return "Hello World"

    @strawberry.field
    def hello2(self, id: strawberry.ID) -> str:
        return "Hello World"


schema = strawberry.Schema(
    Query,
    scalar_overrides={GlobalID: RelayID},
)

This results in

ScalarAlreadyRegisteredError: Scalar `ID` has already been registered

when trying to export the schema.

This is a pretty painful issue for us preventing us from using Relay normally.

axiomofjoy avatar Mar 16 '25 22:03 axiomofjoy

@bellini666 can we work on this on tuesday? 😊

patrick91 avatar May 07 '25 18:05 patrick91

@bellini666 can we work on this on tuesday? 😊

@patrick91 Happy to provide feedback on a real project if that helps. This issue is top of mind for us since we are running into Relay caching issues due to the non-standard ID type. It's forcing us to do manual refetches and incur complexity in our components that otherwise wouldn't be necessary.

I may have a workaround to get around the issue I described above, but it feels sort of hacky.

Image

axiomofjoy avatar May 07 '25 21:05 axiomofjoy

@axiomofjoy yes and that will break with the next version of GraphQL-core :(

My plan of action is to change relay to just use standard ids, provide some GlobalID types for easier migration (with a codemod too) and then deprecate GlobalID potentially?

so current code would become:

class User(NodeWithGlobalID): ...

something like that 😊 (and Node would be what you want, removing the need for scalar overrides)

patrick91 avatar May 07 '25 21:05 patrick91

@axiomofjoy yes and that will break with the next version of GraphQL-core :(

My plan of action is to change relay to just use standard ids, provide some GlobalID types for easier migration (with a codemod too) and then deprecate GlobalID potentially?

so current code would become:

class User(NodeWithGlobalID): ...

something like that 😊 (and Node would be what you want, removing the need for scalar overrides)

Got it, thanks for the info! Just want to check my understanding. It sounds like this approach would require us to switch our node types to subclass NodeWithGlobalID in the short term. Types defined in this way would still have ID fields typed with GlobalID, but would appear in the exported GraphQL schema with ID type instead of GlobalID. In the long-term, it sounds like you want to encourage the pattern of defining types as subclasses of strawberry.relay.Node having strawberry.ID fields. I am assuming this would push the responsibility of manually decoding and parsing Relay IDs into each resolver. Does that sound like what you have in mind?

axiomofjoy avatar May 07 '25 22:05 axiomofjoy

Got it, thanks for the info! Just want to check my understanding. It sounds like this approach would require us to switch our node types to subclass NodeWithGlobalID in the short term.

Ideally yes, but if want to use normal IDs, the you should just need to remove the scalar overrides

At least that's my hope :D

Types defined in this way would still have ID fields typed with GlobalID, but would appear in the exported GraphQL schema with ID type instead of GlobalID.

If you use NodeWithGlobalID you get the GlobalID type (same as now)

In the long-term, it sounds like you want to encourage the pattern of defining types as subclasses of strawberry.relay.Node having strawberry.ID fields. I am assuming this would push the responsibility of manually decoding and parsing Relay IDs into each resolver. Does that sound like what you have in mind?

So, that I'm not sure right now, I think we can still keep strawberry.relay.NodeId, and use that signal strawberry to do the conversion, not sure how just yet though

patrick91 avatar May 07 '25 22:05 patrick91

Got it, thanks for the info! Just want to check my understanding. It sounds like this approach would require us to switch our node types to subclass NodeWithGlobalID in the short term.

Ideally yes, but if want to use normal IDs, the you should just need to remove the scalar overrides

At least that's my hope :D

Types defined in this way would still have ID fields typed with GlobalID, but would appear in the exported GraphQL schema with ID type instead of GlobalID.

If you use NodeWithGlobalID you get the GlobalID type (same as now)

In the long-term, it sounds like you want to encourage the pattern of defining types as subclasses of strawberry.relay.Node having strawberry.ID fields. I am assuming this would push the responsibility of manually decoding and parsing Relay IDs into each resolver. Does that sound like what you have in mind?

So, that I'm not sure right now, I think we can still keep strawberry.relay.NodeId, and use that signal strawberry to do the conversion, not sure how just yet though

Got it, thanks for the clarification. It sounds like the proposal is for types subclassing strawberry.relay.Node with strawberry.relay.GlobalID fields to produce ID types in the exported GraphQL schema going forward. That sounds like it would achieve the result we're looking for with no changes to our application code.

One note I would make is that strawberry.relay.GlobalID is deeply embedded in our codebase at this point as both an input and output type in our resolvers and as a general utility for handling the serialization, deserialization, and parsing of Relay IDs. Part of the challenge for us would be that we have GlobalID in many return types, even ones that do not implement the node interface (one of many examples here). I would also hope for those types to produce ID types in the exported schema as well, i.e., I would like the exported schema to no longer have a custom GlobalID scalar at all.

axiomofjoy avatar May 07 '25 22:05 axiomofjoy

@axiomofjoy thanks for the feedback! yes we can find ways to do that

maybe there's a nicer, simpler approach where we have a config option that maps all GlobalIDs to just IDs (while keeping the current behaviour of conversion)

patrick91 avatar May 07 '25 23:05 patrick91

@axiomofjoy thanks for the feedback! yes we can find ways to do that

maybe there's a nicer, simpler approach where we have a config option that maps all GlobalIDs to just IDs (while keeping the current behaviour of conversion)

That sounds really promising.

axiomofjoy avatar May 08 '25 01:05 axiomofjoy

this is awesome! thanks a lot @patrick91 and @bellini666 ☺️

aryaniyaps avatar May 12 '25 15:05 aryaniyaps

Leaving this here for anyone that have client code still referencing GlobalID, but want to upgrade strawberry to >=0.268.0 and transition to using ID instead of GlobalID smoothly.

Basically replacing all GlobalID strings to ID in the query string, so that old client code will still work, while we update new client code to use ID instead. Yes it's hacky so if anyone has a better way to do a non-breaking migration feel free to let me know.

import json
from strawberry.django.views import AsyncGraphQLView

class CustomGraphQLView(AsyncGraphQLView):
    def get_context(self, request, **kwargs):
        return super().get_context(request, **kwargs)

    def decode_json(self, data):
        # Temporary workaround. We need to replace GlobalID with ID in the query
        # since strawberry published a breaking change to switch from GlobalID to ID
        # Related issue: https://github.com/strawberry-graphql/strawberry/issues/3551
        # We should remove this hacky fix after changing GlobalID to ID on client side
        data_dict = json.loads(data)
        data_dict["query"] = data_dict["query"].replace("GlobalID", "ID")
        return data_dict

lerkstaffie avatar Aug 11 '25 09:08 lerkstaffie

@lerkstaffie you can also just rename the ID type to GlobalID when updating the schema on the client side. This however will not work with operations that use variables of type GlobalID, as that type won't exist on the backend anymore.

Consider this

query(variableX: GlobalID) # would break in validation, as now ID is expected

erikwrede avatar Aug 11 '25 09:08 erikwrede

@lerkstaffie you can also do this for now:

schema = strawberry.Schema(
    query=Query, config=StrawberryConfig(relay_use_legacy_global_id=True)
)

Which should continue using GlobalID, until you finish the migration :)

bellini666 avatar Aug 14 '25 21:08 bellini666

Thanks @erikwrede and @bellini666

I'm aware of the config flag actually. Perhaps I should have clarified betterβ€”the "hacky" temporary workaround is actually for anyone that has to maintain backwards compatibility while migrating (assuming one does not wish to stay on legacy global id forever too).

For example, as my GraphQL API is consumed by iOS and Android mobile apps, any updates to the mobile apps would typically take at least 2-3 days for the user population to update. This means that at one point in time, there may be some clients that are still referencing GlobalID (old app versions), while the new app version targeting ID is being rolled out. The solution merely allows us to transition to using ID which seems to be the more "correct" approach, while not breaking compatibility with the backend code.

lerkstaffie avatar Aug 15 '25 02:08 lerkstaffie