Skip to content

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Mar 5, 2025

Supersedes #6352
See #6332
Spec PR apollographql/specs#58

Add @map and @mapTo directives:

extend schema @link(url: "https://specs.apollo.dev/kotlin_labs/v0.5/", import: ["@map", "@mapTo"])

# Maps to an existing class
extend scalar Date @map(to: "kotlinx.datetime.Instant", with: "com.apollographql.adapters.InstantAdapter")

# Maps to a built-in class (possibly generating an inline class)
extend scalar Length @mapTo(builtIn: Long)

I'm not 100% sold on the names yet, feedbacks welcome.

Under the hood, it moves all the scalar configuration to the schema so we don't have to carry around a ScalarInfo struct. The target & adapter information is present directly in the IR.

Moving forward, I'd love to move most of the compiler configuration to extra.graphqls. That would make maintaining a maven/bazel plugin a lot simpler.

@martinbonnin martinbonnin requested a review from BoD as a code owner March 5, 2025 12:39
@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Mar 5, 2025

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: e6b078b0dafd328e667e29b9

@martinbonnin martinbonnin force-pushed the move-scalars-to-schema branch from 33949b5 to 3b88623 Compare March 5, 2025 12:39
@martinbonnin martinbonnin added this to the 4.2.0 milestone Mar 5, 2025
Copy link
Contributor

@BoD BoD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉🎉🎉

Re: naming, the only alternative I can think of is @mapBuiltin(to: instead of @mapTo(builtIn: which would be a bit more symmetrical to @map(to: - but no strong preference.

I also wonder if type / adapter would slightly be less potentially confusing than to/ with , but again not sure and either sound good to me.

@martinbonnin
Copy link
Contributor Author

@mapBuiltin(to:

This reads aloud as "map built-in to Foo" which can be a bit confusing. We're not mapping a built-in, we are mapping TO a built-in

type / adapter would slightly be less potentially confusing than to/ with

I think if we go that road we would want a noun for the directive name:

extend scalar Timestamp @mapping(type: "kotlinx.datetime.Instant", adapter: "com.apollographql.adapters.InstantAdapter")

But the pattern of using verbs is relatively established now (@catch(to:), @provides(fields:), @override(from:), etc...).

@BoD
Copy link
Contributor

BoD commented Mar 5, 2025

But the pattern of using verbs is relatively established now

Very true, also @skip and @defer.

All good then 👍

@martinbonnin martinbonnin merged commit 635a93b into apollographql:main Mar 5, 2025
5 checks passed
@martinbonnin martinbonnin deleted the move-scalars-to-schema branch March 5, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants