add withScalar method

[Liam-Tait]

Based on this issue #910

Add method withScalars to SchemaBuilder to enable loading scalars and inferring their types for convenience. This especially helps with using existing libraries like graphql-scalars

Loading scalars is done via a method instead of as an option to enable the inference to merge the Scalars from the manually typed scalars and the inferred scalars. This is because typescript doesn't have a way to do partial inference.

Previously

const builder = new SchemaBuilder<{
  Scalars: {
    Date: {
      Input: Date;
      Output: Date;
    };
  };
}>({});

builder.addScalarType('Date', CustomDateScalar, {});

Using withScalars

const builder = new SchemaBuilder({}).withScalars({ Date: CustomDateScalar });

[changeset-bot]

🦋 Changeset detected

Latest commit: 6461446

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages

Name	Type
@pothos/core	Minor
@pothos-examples/envelope-helix-fastify	Patch
@pothos-examples/graphql-shield	Patch
@pothos-examples/helix	Patch
@pothos-examples/nextjs	Patch
@pothos-examples/open-telemetry	Patch
@pothos-examples/prisma-federation	Patch
@pothos-examples/prisma-smart-subscriptions-apollo	Patch
@pothos-examples/prisma-subscriptions	Patch
@pothos-examples/prisma	Patch
@pothos-examples/relay-windowed-pagination	Patch
@pothos-examples/simple-classes	Patch
@pothos-examples/simple-interfaces	Patch
@pothos/website	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
pothos	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 19, 2023 10:01am

[Liam-Tait]

@hayes This is ready except for the documentation. How do you want to prioritise the withScalars approach vs manual typed in the documentation?

[hayes]

Thanks so much for working on this change!

Added a few notes and questions, but generally looks like the right direction!

Regarding documentation, I think I'd prefer to keep recommending the current approach as the default until: this adds helpers for defining custom scalar fields, and graphql-scalars publishes a version where the scalars all have types

Sorry it's taking me a while to get this review done, and let me know if anything is unclear. Looking at this on my phone with spotty service, so probably not the best conditions for a good review

[hayes]

I know it's a bit awkward and inconvenient, but it's pretty important to be able to provide these options.

This could be Date: [DateResolver, options] or Date: { type/resolver/implementation: DateResolver, ...otherOptions }

Personally I like the second option (although the resolver terminology from graphql-scalars is not my favorite.

I'd probably make it work so you can pass either just the scalar, or scalar + options. Inference will be slightly more complex but shouldn't be too bad.

[Liam-Tait]

I'm working on this, might take a bit of time

I am aiming to have the following be valid.

const builder = new SchemaBuilder({}).withScalars({
  Date: DateScalar,
  UUID: { scalar: UUIDScalar },
  PositiveInt: {
    scalar: PositiveIntScalar,
    options: {
      extensions: { codegenScalarType: "number" },
    },
  },
});

Does that look good to you?

[Liam-Tait]

The pr to expose TInternal and TExternal from graphql-scalars has been merged, so should be in the next release of graphql-scalars

[hayes]

@Liam-Tait been a busy few weeks for me, sorry I haven't checked in here in a bit. Do you want me to clean this up and get it merged?

[Liam-Tait]

@hayes no rush at all. Happy for you to pick this up if you would like to. I believe changes required are adding options to each scalar.

My company works in cycles, I’ll have time to continue work on this after 7th July when we are in a “cooldown” mode if you are happy to wait that long

[hayes]

If I end up having some time, I'll try to work on this, but no rush on my end either, might just leave it for for you! Summers tend to get pretty busy for me, and I have less time outside work for hacking on side projects.

[Liam-Tait]

I have updated this pr to have many chained calls to withScalar(name, scalar, options)

For example using many graphql-scalars

import {
  GraphQLEmailAddress,
  GraphQLHexColorCode,
  GraphQLBoolean,
  GraphQLISBN,
  GraphQLBigInt,
  GraphQLRGB,
  GraphQLDate,
  GraphQLJSONObject
} from 'graphql-scalars';
import SchemaBuilder from '@pothos/core';

const builder = new SchemaBuilder({})
  .withScalar('Email', GraphQLEmailAddress)
  .withScalar('HexColor', GraphQLHexColorCode)
  .withScalar('Boolean', GraphQLBoolean)
  .withScalar('ISBN', GraphQLISBN)
  .withScalar("BigInt", GraphQLBigInt)
  .withScalar("RGB", GraphQLRGB)
  .withScalar("Date", GraphQLDate)
  .withScalar("JSONObject", GraphQLJSONObject)

[Liam-Tait]

@hayes Are you happy with the chaining withScalar instead of withScalars taking an object?

[hayes]

Hey, sorry I missed your last update! Chaining APIs create a lot of complexity in the type-system, and I am not sure how this would affect the builder types. Early versions of Pothos experimented with using chaining APIs to define fields, the issue was that you could very easily hit max stack depth limits in the type-system because each chained API is dependent on the result of the previous call. Similarly, this API would make every builder method dependent on a chain of type-transforms. I am not sure if this would actually cause issues in the real world, but is definitely something I would be concerned about, and would need to explore in depth before adding an API like that.