bdelacretaz commented on pull request #1:
URL: 
https://github.com/apache/sling-org-apache-sling-graphql-core/pull/1#issuecomment-651796131


   > I guess we could register multiple instance of the same 
SlingScalarConverter-implementing wrapper component (one for each scalar) 
manually within the adapter, for example, but all the additional 
lifecycle-handling (when to register/unregister) frightens me). But from our 
perspective the proposed solution would be the simplest.
   
   In the end I think this is the same as the provider service that you are 
suggesting: you end up with a wrapper for each scalar converter that adapts it 
to the Sling API.
   
   The lifecycle handling is not complicated if you need to wrap "arbitrary 
scalar converters": just register an OSGi `SlingScalarProvider` service for 
each and you're good. This can be done either with Declarative Services 
annotations or (probably more convenient in this case) by registering the 
wrapper services using `BundleContext.registerService.`
   
   I'm happy to create an example of that, in this module's test code for 
example, to show how it can work. I think you'll find that it's the same as 
implementing your scalars provider, except that the provider registers OSGi 
services instead of making the converters available via an API that we can 
avoid. I'd say we get the same result but with less code and a smaller API 
surface.
   
   > IMHO scalars should not be defined globally, but per endpoint. Typically, 
they are part of the schema...
   
   I agree that the set of active scalars is often schema-dependent. We might 
use a `@namespace(scalars="mynamespace")` directive or something similar to 
namespace them. I think that's an orthogonal concern to what we are discussing 
here.
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to