Scalar decoders

[dillonkearns]

This is a brainstorm on how to have user defined Json Decoders for Scalar values.

Proposed Process

1. Call @dillonkearns/elm-graphql NPM executable, and include a flag to tell it a user-defined module to use for the decoders. Something like elm-graphql --scalar-decoders MyScalarDecodersModule
2. In order to avoid a confusing and bug-prone user experience around initially generating updating the user-defined ScalarDecoders.elm module, the Api.Scalar module will contain a comment that is a valid module. You can copy-paste that module to have a valid starting point.
3. The rules for a valid user-defined module are as follows (see ScalarDecoders.elm in this pull request for a valid example):
   It must include:
   • A type alias for every scalar in your GraphQL Schema, for example type alias PosixTime = Time.Posix. These type aliases will be used in the type annotations in the generated code.
   • The module must expose a function called decoders
     -EDIT: The latest prototype is using an opaque type, so the type annotation is no longer required (see this commit). decoders must be annotated with the Scalar.Decoders type alias. This way it will break in the user-defined code in case your schema changes, rather than giving you compilation errors from the generated code which would be misleading.

If you call the CLI without a --scalar-decoders option, then it will use Scalar.defaultDecoders.decoderPosixTime (or .decoder<SomeOtherScalar>, etc.) to decode all scalars. See defaultDecoders in the Swapi.Scalar module in the prototype code in this pull request.

Areas for Feedback

• How does the comment in <ApiBaseModule>.Scalar sound as a strategy for avoiding doing code generation to tweak a user-maintained file? I'm open to other ideas, but using codegen to touch a file maintained by the user seems like something we should avoid.

• How does the experience seem for using defaults (see decoderId in ScalarDecoders.elm in this pull request)? Any ideas on how to improve that? I don't think there's a way to use record update syntax because you can change the type signature compared with defaultDecoders based on the types of your decoders. Maybe there could be a nicer syntax if a function was used here?

• Will the error messages from the Elm compiler be misleading if you forget to define a type alias for a given scalar? Is there a more elegant way to do this?

These points are no longer relevant

• The Api.Scalar.Decoders type alias has type variables for each decoder. This works fine, but it seems like it could be a bit pain for the user to maintain this since the type variables are positional (not key-value pairs), so it's hard to know which one to update. You can always copy-paste the type annotation from the generated comment in Api.Scalar any time it changes. Maybe there's an opportunity for improvement here? This commit uses an opaque type not a type alias. So you can actually skip the type annotation safely here if you want to.
• How do we enforce that the user uses an annotation on this type? It seems pretty awkward to enforce this by doing a string search on the module (and looking up the module would be a pain because we would need to look at the source paths in the elm.json). Maybe a function would help here, too. That is, make it an opaque type instead of a type alias, and the only way to create it is by calling a function. So the user-maintained code would just call that function, and that would give more control and ensure that the compilation errors happen in the users code, not the generated code, if they have a mistake in the user-maintained module. This is no longer relevant with the opaque type.

[dillonkearns]

For the purposes of showing this prototype, I changed this to reference Query.now. Since it's using a custom decoder, the type of Query.now is SelectionSet Time.Posix RootQuery.

[BrianHicks]

this defaultDecoders business is tingling my spidey senses. To use a default, you have to know the default exists.

Would it be possible to require each scalar to have a separate module? Separating data structures is what modules do best! So then:

Module	Has
Scalar.Id	type alias Id, decode : Decoder Id
Scalar.PosixTime	type alias PosixTime, decode : Decoder PosixTime

Figuring out what default to set could be tricky, but maybe there could be a command-line flag for that? Or some GraphQL documentation annotation which generated the right thing?

[BrianHicks]

oh duh, easier way to do the right thing: write a default scalar value to the filesystem when it's missing and prompt the user to customize it. It's easy to assume they're all strings by default, and this way you can have some inline documentation on how to modify the code in place rather than having to copy or construct something from scratch.

[dillonkearns]

@BrianHicks thanks for the comments! I like the idea of having a file per scalar, that seems like an interesting path to explore.

Any ideas on how we could use a single-file-per-scalar approach and maintain these characteristics?

• The user should get a compiler error pointing to their custom scalar code (not the generated code) if they have a missing API.
• If a Scalar is removed or renamed, the user should get an error for referencing it (as opposed to just having a file that's not referenced anywhere and not getting an error)
• The code generator shouldn't have to touch or read files that the user created or changed (this makes things way simpler to maintain and less bug-prone for the codegen, and is a nice sort of dependency inversion thing I think. I also think it leads to a simpler UX)

[BrianHicks]

The user should get a compiler error pointing to their custom scalar code (not the generated code) if they have a missing API.

fill in a blank and tell them at generation time? That way if you delete a thing, it's kinda obvious what's going on. Not as good as the system you specified above but still maybe OK for other tradeoffy reasons like better organization.

If a Scalar is removed or renamed, the user should get an error for referencing it (as opposed to just having a file that's not referenced anywhere and not getting an error)

Elm will generate an error for this, but not a good one for the use case, you're right.

The code generator shouldn't have to touch or read files that the user created or changed (this makes things way simpler to maintain and less bug-prone for the codegen, and is a nice sort of dependency inversion thing I think. I also think it leads to a simpler UX)

What I'm proposing is a write-if-missing implementation, which means you'd reference stuff but not read/touch it from codegen otherwise.

---

Those feel like pretty weak answers to some good questions, which has revealed flaws! I'm still not 100% sure on lumping them into the same module but it seems more fine now. ;)

[dillonkearns]

Yeah, leveraging the Elm compiler as much as possible has proven to be a great guiding principle. I think it very much applies here. Any place where we would need to manually check something and provide an error message to me is an indication that the design needs to be refined to find a way to get Elm to tell the user what the problem is instead.

I think the single file approach is nice in that regard. And the opaque type idea helps with that as well. The only way to have it be the right type is to call a function, and you will get a compiler error in the user code which is the origin of the problem.

[dillonkearns]

The single file idea could still be fruitful, though, I just haven't thought of how to refine it in that way! Either way, I appreciate the discussion, I wouldn't have thought of that approach.

It could be that the single file approach isn't as convenient if it leverages the Elm compiler, though, because you would need to call a function in each file to get those nice compiler errors, instead of just in a single file.