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

Custom Scalar Support #433

Closed
scottrabara opened this issue Dec 27, 2018 · 9 comments
Closed

Custom Scalar Support #433

scottrabara opened this issue Dec 27, 2018 · 9 comments
Assignees
Milestone

Comments

@scottrabara
Copy link
Contributor

Hi, I've been using the HotChocolate library for some time now and am getting around to trying out custom scalar types. When a scalar type is given a unique name such as "CustomLong", it can be registered just fine and used within my other types as input/output.

public class CustomLongType : ScalarType
    {
        public CustomLongType() : base("CustomLong")
        {
        
        }
        ...
    }

Via your documentation, it mentions the ability to swap out the built-in base Scalar types. I have tried to swap out your guys' long scalar type to be able to add my own custom behavior, but have not been able to do so. Defining my CustomLongType like such:

public class CustomLongType : ScalarType
    {
        public CustomLongType() : base("Long")
        {
        
        }
        ...
    }

This seems to fail to register the type correctly in TypeRegistry as the _namedTypes dictionary will pull back the built-in ScalarType for long {HotChocolate.Types.LongType}

private void TryUpdateNamedType(INamedType namedType)
{
	INamedType namedTypeRef = namedType;

	if (!_namedTypes.TryGetValue(namedType.Name, out namedTypeRef))
	{
		namedTypeRef = namedType;
		_namedTypes[namedTypeRef.Name] = namedTypeRef;
	}
        ...
}

Should there be something here to check for IsScalarType to swap with?

Is there a better way to do such swapping? Is this type of swap allowed? Let me know if I am doing something completely wrong!

@michaelstaib michaelstaib added the ❓ question This issue is a question about feature of Hot Chocolate. label Dec 27, 2018
@michaelstaib michaelstaib self-assigned this Dec 27, 2018
@michaelstaib
Copy link
Member

Hi Scott,

I will have a look at that sometime tonight.

@michaelstaib
Copy link
Member

OK, got it already. This is a design flaw actually since we bundled the extended scalar types with the core scalar types.

The core scalar types are the ones that are defined by the spec and cannot be changed.

So, we will fix with the next version 0.7.0 which should arrive beginning of January. I will add this item to the 0.7.0 backlog.

We will make a preview build that includes this fix an notify you once it is ready.

@michaelstaib michaelstaib added bug and removed ❓ question This issue is a question about feature of Hot Chocolate. labels Dec 27, 2018
@michaelstaib michaelstaib added this to the 0.7.0 milestone Dec 27, 2018
@scottrabara
Copy link
Contributor Author

Awesome thanks Michael for the quick response!

@michaelstaib
Copy link
Member

Hi Scott @scottrabara,

I will leave this issue open and close it once the fix is included in a preview build.

@michaelstaib
Copy link
Member

We also have a slack channel for questions etc ...

https://join.slack.com/t/hotchocolategraphql/shared_invite/enQtNTA4NjA0ODYwOTQ0LTBkZjNjZWIzMmNlZjQ5MDQyNDNjMmY3NzYzZjgyYTVmZDU2YjVmNDlhNjNlNTk2ZWRiYzIxMTkwYzA4ODA5Yzg

@michaelstaib
Copy link
Member

Hey Scott,

I will move this one to Version 0.8.0. In version 0.8.0 we have a lot of schema enhancements and I think this is a better fit for that release. 0.7.0 is more about middleware, execution engine and co.

@michaelstaib michaelstaib modified the milestones: 0.7.0, 0.8.0 Jan 3, 2019
@michaelstaib michaelstaib added 🙋 help wanted Extra attention is needed 🙋 good first issue Good for newcomers and removed 🙋 good first issue Good for newcomers labels Jan 3, 2019
@michaelstaib michaelstaib added in progress and removed 🙋 help wanted Extra attention is needed labels Jan 4, 2019
@michaelstaib
Copy link
Member

michaelstaib commented Jan 4, 2019

The scalar types are registered by the SchemaContextFactory. The schema context is the internal initialization context for the schema.

The factory has two functions that preregister scalar types (RegisterSpecScalarTypes and RegisterSpecScalarTypes).

All types that are preregistered by the SchemaContextFactory cannot be changed. This behavior is correct since we do not want people to meddle with the spec scalars or the introspection types.

What is wrong is to register the extended scalars here.

So my suggestion is to remove the RegisterExtendedScalarTypes from that class and create a new extensions method in SchemaConfigurationExtensions called RegisterExtendedScalarTypes that registers those types.

With that users of the API could register the package with one line of code and use HC like before or the could register just some of those types by manually registering them.

Tests

  • Test that shows that the extended types are not registered without calling RegisterExtendedScalarTypes on ISchemaConfiguration
  • Test that proves that custom scalars for can be registered.
  • Test that proves that the spec scalars cannot be swapped out.
  • Test that shows that all extended types are correctly registered when RegisterExtendedScalarTypes on ISchemaConfiguration is called.

Changelog

  • Update the change log ([Unreleased] section)

Documentation

@scottrabara
Copy link
Contributor Author

@michaelstaib Documentation and changelog have been updated, please see PR's referencing this issue above.

michaelstaib pushed a commit that referenced this issue Jan 6, 2019
@michaelstaib
Copy link
Member

@scottrabara I will close this one since everything is implemented.

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

No branches or pull requests

2 participants