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: [email protected]
