[Typescript] Octokit "auth" option does not support example from docs for installation tokens

[douglascayers]

I recently updated the following octokit modules and began receiving a deprecation warning about not providing an authStrategy.

Deprecation Warning

Setting the "new Octokit({ auth })" option to an object without also setting the "authStrategy" option is deprecated and will be removed in v17. See (https://octokit.github.io/rest.js/#authentication)

Octokit Modules

"@octokit/app": "4.1.1",
"@octokit/auth-app": "2.4.2",
"@octokit/plugin-retry": "3.0.1",
"@octokit/plugin-throttling": "3.2.0",
"@octokit/rest": "16.40.1",
"@octokit/webhooks": "7.0.0",

Code That Raises Deprecation Warning

import OctokitRest from '@octokit/rest';

const client = new OctokitRest({
    async auth(): Promise<string> {
        const token = await app.getInstallationAccessToken({
            installationId: MY_INSTALLATION_ID,
        });
        return `token ${token}`;
    },
    ...
});

Code I Would Like To Use

import { createAppAuth } from '@octokit/auth-app';
import OctokitRest from '@octokit/rest';

const client = new OctokitRest({
    authStrategy: createAppAuth,
    auth: {
        id: MY_APP_ID,
        privateKey: MY_PRIVATE_KEY,
        installationId: MY_INSTALLATION_ID
    },
    ...
});

Discrepancy to Documentation

Per the example in the documentation, I should be able to specify the app id and privateKey as the auth settings but those are not valid per the current typescript definition.

Proposal

Not sure if this is on your roadmap to v17, but if not then here's my two cents.

Since the octokit rest module's authentication plugin just passes options.auth to the auth strategy via const auth = options.authStrategy(options.auth);, should the typescript definition for auth in this octokit rest module match StrategyOptions type as shown in the auth-app usages?

Thanks!

[gr2m]

Thank you for reporting the bug Doug.

v17 of @octokit/rest will be built upon https://github.com/octokit/core.js. I tried to come up with a helpful TypeScript definition where auth types depend on the value of authStrategy, but to no avail. It does not seem that TypeScript currently supports that. Here is my question on StackOverflow on that: https://stackoverflow.com/q/59828577/206879

So for now, I will set auth to any, unless you have a better idea?

[gr2m]

Could you have a glance at my PR at https://github.com/octokit/rest.js/pull/1563? It should remove the TypeScript error you are seeing?

[douglascayers]

Thanks for the quick follow up @gr2m. I’ll try to review the PR this week. Just wanted to reply to say I saw your message and on my todo to follow up. Thanks

—

Yes, the addition of any type will remove the compilation error. It gives us time in the short term to explore how to provide type checking in a future update.

[gr2m]

Thanks Doug, I'll go ahead and merge it then. I'd love to improve the Types for the auth option. Please let me know if you have any ideas

[gr2m]

fixed in v16.40.2

[douglascayers]

Thanks!

[weswigham]

@gr2m all you want is a union type for Options, rather than for the auth field alone, so you can tie the types of the authStrategy and auth fields together.

Something like

  
  export interface AuthStrategyOne {
    authStrategy: AnAuthStrategy;
    auth: OptionsAnAuthStrategy;
  }
  export interface AuthStrategyTwo {
    authStrategy: TheThing;
    auth: OptionsForTheThing;
  }
  export interface NoAuthStrategy {
    authStrategy?: undefined;
  }
  export type Options = BaseOptions & (AuthStrategyOne | AuthStrategyTwo | NoAuthStrategy);
  export interface BaseOptions {
    userAgent?: string;
    previews?: string[];
    baseUrl?: string;
    log?: {
      debug?: (message: string, info?: object) => void;
      info?: (message: string, info?: object) => void;
      warn?: (message: string, info?: object) => void;
      error?: (message: string, info?: object) => void;
    };
    request?: {
      agent?: http.Agent;
      timeout?: number;
    };
    /**
     * @deprecated Use {request: {timeout}} instead. See https://github.com/octokit/request.js#request
     */
    timeout?: number;
    /**
     * @deprecated Use {userAgent, previews} instead. See https://github.com/octokit/request.js#request
     */
    headers?: { [header: string]: any };
    /**
     * @deprecated Use {request: {agent}} instead. See https://github.com/octokit/request.js#request
     */
    agent?: http.Agent;
    [option: string]: any;
  }

[gr2m]

Unfortunately we don't know the full set of authentication strategies. Users can build their own. But I guess we could have some kind of fallback if it doesn't fit any of the known strategies?

[weswigham]

Or you could make a sort of strategy registry;

export interface OctokitAuthStrategies {
  AuthStrategyOne: {
    authStrategy: AnAuthStrategy;
    auth: OptionsAnAuthStrategy;
  }
  AuthStrategyTwo: {
    authStrategy: TheThing;
    auth: OptionsForTheThing;
  }
  NoAuthStrategy: {
    authStrategy?: undefined;
  }
}
export type Options = BaseOptions & (OctokitAuthStrategies[keyof OctokitAuthStrategies]);

which can then be augmented by consumers/strategy authors with

declare module "@octokit/rest" {
  export interface OctokitAuthStrategies {
    MyCustomStrategy: {
      authStrategy: CustomThing;
      auth: CustomOptionsThing;
    }
  }
}

[gr2m]

I see, thank you! Didn't think about that possibility!

[gr2m]

This would need to be implemented in @octokit/core, the upcoming version of @octokit/rest will be built on top of it. If anyone wants to beat me to it, please go ahead. I'll comment here once I start working on it myself

[gr2m]

closing in favor of octokit/core.js#323