TypeScript: SCIMMYRouters does not extend express.Router

[fflorent]

Despite SCIMMYRouters extending express.Router in the JavaScript source, this is not the case in the generated Typescript type definitions:

    /**
     * Method invoked to determine a base URI for location properties in a SCIM response
     * @callback AuthenticationBaseUri
     * @param {express.Request} req - the express request to provide the base URI for
     * @returns {String|Promise<String>} the base URI to use for location properties in SCIM responses
     * @private
     */
    /**
     * SCIMMY HTTP Routers Class
     * @class SCIMMYRouters
     */
    export default class SCIMMYRouters {
        /**
         * Construct a new instance of SCIMMYRouters, validate authentication scheme, and set SCIM Service Provider Configuration
         * @param {Object} authScheme - details of the means of authenticating SCIM requests
         * @param {String} authScheme.type - SCIM service provider authentication scheme type
         * @param {AuthenticationHandler} authScheme.handler - method to invoke to authenticate SCIM requests
         * @param {AuthenticationContext} [authScheme.context] - method to invoke to evaluate context passed to SCIMMY handlers
         * @param {AuthenticationBaseUri} [authScheme.baseUri] - method to invoke to determine the URL to use as the base URI for any location properties in responses
         * @param {String} [authScheme.docUri] - URL to use as documentation URI for service provider authentication scheme
         */
        constructor(authScheme?: {
            type: string;
            handler: AuthenticationHandler;
            context?: AuthenticationContext;
            baseUri?: AuthenticationBaseUri;
            docUri?: string;
        });
    }

This forces to cast any instance of SCIMMYRouters into express.Router when trying to use it.

NB: I have quickly tested to add an @extend JS annotation, I did not work. I open this issue because I am a bit in a hurry but I will be glad to fix it myself if you have no time for that either.

[sleelin]

Hi @fflorent,
Since the source files are written in JavaScript, the type declarations are automatically generated by the TypeScript compiler from the JSDoc comments. Unfortunately, the TypeScript compiler is actually quite limited in its handling of declaration generation from JSDoc comments, and does not seem to retain the express.Router extension (despite explicitly specifying it with the @extends tag as you mentioned). As a test, when I manually added the missing extends express.Router to the generated declaration, TypeScript then complained about express.Router not being a constructable class or function - despite this being perfectly acceptable in JavaScript.

The solution involved declaring an intermediary interface that extends express.Router, and adding the @implements JSDoc tag (which is retained by TypeScript) to the SCIMMYRouters class. This was defined in a preauthored routers.d.ts file that is merged with the generated declaration file. Testing locally, this seemed to appease TypeScript, which then seemed to recognise SCIMMYRouters instances as suitably extending the express.Router type.

This change should be available in the next release (v1.2.2) - please let me know if this resolves the issue for you.

Thanks,
Sam

[fflorent]

Hi @sleelin,

Sorry for the late reply and thank you!

Unfortunately, I could not remove the casting here despite having upgraded to v1.2.2:

https://github.com/gristlabs/grist-core/blob/7ace7f80acf292b8e22b337bb01b64676059d501/app/server/lib/scim/v2/ScimV2Api.ts#L47

(currently in this PR: gristlabs/grist-core#1199)

[sleelin]

Thanks for following up @fflorent
It looks like there's a discrepancy between how the TypeScript compiler handles imported type references in JSDoc comments - when I originally tested, it was automatically adding this line at the start of the generated declarations, so I opted to omit it when merging the declaration file. It seems that didn't actually happen when the release build ran, and doesn't seem to be happening in my testing now either 😕

Since there's no reference to the express module, TypeScript doesn't recognise what the SCIMMYRouters interface is extending, which is why you still needed to cast it. I'm reopening the issue as it's clearly not resolved, but matters are also complicated by the fact that the express types aren't actually very useful here - the express.Router "class" which is extended does some magic that makes TypeScript not recognise it as a class that can be directly extended, and technically SCIMMYRouters should actually be exposing a method that returns a router instead of being an instantiable class itself.

[sleelin]

Hi @fflorent,
I've just published v1.3.1 of scimmy-routers which should finally solve this issue and allow you to use the SCIMMYRouters instance without needing to cast it to express.Router!
If possible, please update to this version and report back as to whether this is now properly resolved.

Thanks,
Sam