Skip to content
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

[2022-07-07] Open a discussion thread about meta-fields proposal #1055

Closed
benjie opened this issue Jul 8, 2022 · 10 comments
Closed

[2022-07-07] Open a discussion thread about meta-fields proposal #1055

benjie opened this issue Jul 8, 2022 · 10 comments

Comments

@benjie
Copy link
Member

benjie commented Jul 8, 2022

Presentation: https://docs.google.com/presentation/d/1qGc2rZKeim699tT-2G1xDESZP3Kd6tvj9V27tQfyJhg/edit#slide=id.ge9d8d484d6_0_5

Lee: I agree with a lot of the ideas and problem statements from both presentations. To be respectful for time, we should have a discussion thread to start discussing this async
ACTION - Ivan - open discussion thread.

Video link to beginning of Ivan's topic: https://youtu.be/gVCszBu_aJI?t=2714

And to where the action items are discussed: https://youtu.be/gVCszBu_aJI?t=4850


Note: Action Item issues are reviewed and closed during Working Group
meetings.

@benjie benjie added this to To do in Actions 2022-07-07 via automation Jul 8, 2022
@nodkz
Copy link
Contributor

nodkz commented Jul 8, 2022

@rivantsov @IvanGoncharov i want to add a note about directives, that we have 2 types of directives

  • schema definition directive of two sub-types
    • meta-field directive, eg deprecated
    • schema construction instructions eg entity, key, relation, permission, etc
  • runtime directives -- skip, if, lowercase

So maybe better to add to directives a kind/type property:

  • runtime (for request language) if, skip
  • public-meta (for client information) deprecated
  • generation (only server side not exposed via introspection) key, external, permission, relation.

Of course these directives type/kind names are debatable. And need to find better names. But from my point of view I see 3 types/kinds of directives which should be clarified in spec for different purposes.

@nodkz
Copy link
Contributor

nodkz commented Jul 8, 2022

BTW oneOf directive has two scopes public-meta and generation.

Ouch, I just found a good word for distinguishing of 3 directive kinds - scope.

@rivantsov
Copy link
Contributor

aren't these dir sub-types already covered by Location?

@nodkz
Copy link
Contributor

nodkz commented Jul 8, 2022

Nope.

  • Location - points on for what kind of type node it applicable (field, type, arg, ...)
  • Scope - will point on what side of schema this directive is applicable (only on server-side or it might be exposed via introspection to the client).

@rivantsov
Copy link
Contributor

oh, no, that's one of the points I argued on several occasions. ALL directives should be exposed. There should be such thing as hidden directives

@nodkz
Copy link
Contributor

nodkz commented Jul 8, 2022

Right now we have a mess in directives. Some directives are used in SDL, others in request language. Some directives are sensitive and should live only on server (eg @cypher neo4j-graphql or @inaccessible in apollo federation)

According to this mess, instead of clarifying of directive scopes we start to invent meta-fields.

If we provide ability to mark directives scope and a way to filter out some of them from introspection we will escape addition of meta-fields.


About backwards compatibility:

directive @some(arg: String) on FIELD

will be acted like

directive @some(arg: String) on FIELD scopes [request, server, client]

Or we may leave just two types of scope for simplicity - [client, server]. But it's bad, because we don't distinguishing meta directives for informing clients as meta-fields (like deprecated) and directives which might be used in GraphQL queries (like skip).

So if developer explicitly provide scopes

directive @cypher(statement: String) on FIELD scopes [server]

then this directive will not be exposed via introspection. Because it does not have client scope.

@LunaticMuch
Copy link
Contributor

@nodkz I disagree in principle, while your solution is not necessarily bad.
The mess is clear, and it's on the specs too which have together @deprecated, an informative metadata with @skip that has an effect.

Specs also say:

Directives provide a way to describe alternate runtime execution and type validation behavior in a GraphQL document.

but also

Directives can be used to describe additional information for types, fields, fragments and operations.

IMO these are conflicting.

@IvanGoncharov has described very well the difference between directives and metadata. Two reflective questions are:

  1. which kind of metadata do I want to add to a type or a field in a real world?
  2. how several directives and several metadata affect the developer experience?

IMO, if I need to add all the directives I want to my schema, I would easily and quickly end with 10/15 directive per field. These include and do not limit to:

  • data classification
  • data steward/owner/governance information
  • security classes
  • review/approval info in some cases
  • data dictionary (we might discuss on this, but the dictionary, which could be part of the description, might also want to be out from it keep the description purposefully short to better fit into IDEs and playground)

Moreover, most of them would not be added or managed by a developer, so actually they would require a third party to touch the schema file. Using metadata would allow the schema do be built and managed by an engineer and another party to be able to manage metadata on a different file without impacting the schema.

I see more value in separating the whole metadata piece from the schema, and therefore, in strengthening the idea directives are there for guiding resolvers. Clear separation of concerns.

@benjie
Copy link
Member Author

benjie commented Jul 29, 2022

I agree with pretty much everything @LunaticMuch has said, with the caveat that the metadata is part of the schema: it should be introspectable via the schema introspection, it should be representable in schema SDL, etc. Though you could define the metadata somewhere else during schema building (there are many ways to build a GraphQL schema), ultimately it will end up as part of the final schema. (This may or may not align with @LunaticMuch's intent.)

@LunaticMuch
Copy link
Contributor

@benjie we're aligned. I misrepresented my idea.
I agree metadata must come out from introspection. I do not necessarily agree that directives should (that's the actual behaviour).

Splitting files, it's just a convenient pattern, which is achievable today as well with current tools. Although, given the difference between directives and metadata, your schema should not break if, when it gets built, the metadata for a type is missing. This allow for clear separation.

@benjie
Copy link
Member Author

benjie commented Oct 11, 2023

I could not find the discussion thread for this particular proposal; but there is one for Lee's proposal here: #1096

There's been no activity on this for over a year so I'll close it.

@benjie benjie closed this as completed Oct 11, 2023
Actions 2022-07-07 automation moved this from To do to Done Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

5 participants