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

Support callback for defaults #326

Open
astorije opened this issue Oct 5, 2021 · 7 comments
Open

Support callback for defaults #326

astorije opened this issue Oct 5, 2021 · 7 comments
Labels
Type: Support Any questions, information, or general needs around the SDK or GitHub APIs

Comments

@astorije
Copy link

astorije commented Oct 5, 2021

Hi there!

All the examples of the .defaults() function that I could find are pretty straightforward, for example:

export const graphql = baseGraphql.defaults({
  headers: { authorization: 'token 1234' },
});

However, there are cases where some defaults are not available during the initial importing of modules, for examples if they are loaded from somewhere (i.e. async, e.g. from disk or another API) or if they require additional checking/validation.

Essentially, it would be great if we could do something like this:

export const graphql = baseGraphql.defaults(async () => {
  const oAuthToken = await getOAuthToken();
  return { headers: { authorization: `token ${oAuthToken}` } };
});

The underlying type of defaults would be:

defaults: (newDefaults: RequestParameters | (() => Promise<RequestParameters>)) => graphql;

In the meantime, we could probably get away with something like the following, but TypeScript types are getting in the way and I don't want to end up with something too hacky just to be able to use .defaults:

// This does not work
export const graphql: typeof baseGraphql = async (...args) => {
  const oAuthToken = await getOAuthToken();

  const graphqlWithAuth = baseGraphql.defaults({
    headers: { authorization: `token ${oAuthToken}` },
  });

  return graphqlWithAuth(...args);
};
@gr2m
Copy link
Contributor

gr2m commented Oct 5, 2021

I don't think it's necessary. If you really need a defaults callback then you can use https://github.com/octokit/core.js which does support it.

For your particular case you can set request.hook, which is described in https://github.com/octokit/request.js/#request

image

That's how @octokit/core is hooking into the request lifecycle when using octokit.request()/octokit.graphql(), too.

@gr2m gr2m added the Type: Support Any questions, information, or general needs around the SDK or GitHub APIs label Oct 5, 2021
@astorije
Copy link
Author

Hi @gr2m, thanks for the response!

Do you think you could provide some guidance on how to accomplish that?

That's where I'm at right now and needless to say it's not working + those TS types are definitely wrong...

import { withCustomRequest } from '@octokit/graphql';
import { request } from '@octokit/request';

const myRequest = request.defaults({
  request: {
    async hook(req: any, options: any) {
      const oAuthToken = await getOAuthToken();

      req.headers = { authorization: `token ${oAuthToken}` };

      return req(options);
    },
  },
});

const graphql = withCustomRequest(myRequest);

Thanks!

@gr2m
Copy link
Contributor

gr2m commented Oct 12, 2021

try this

-      req.headers = { authorization: `token ${oAuthToken}` };
+      options.headers.authorization = `token ${oAuthToken}`

@astorije
Copy link
Author

astorije commented Oct 12, 2021 via email

@astorije
Copy link
Author

Alright, so the following works beautifully:

export const graphql = withCustomRequest(
  request.defaults({
    request: {
      async hook(req: any, options: any) {
        const oAuthToken = await getOAuthToken();

        if (!options.headers) {
          options.headers = {};
        }

        options.headers.authorization = `token ${oAuthToken}`;

        return req(options);
      },
    },
  }),
);

Regarding the types:

  • Is req the same type as request? If so that's RequestInterface<object> but it doesn't seem to be exported anywhere
  • It seems like the type of options is RequestParameters which is only exported in @octokit/graphql/dist-types/types, not top-level

Any chance hook could get typed in the library, so TypeScript can infer the type of the callback and the arguments automatically without having to type those by hand?

@astorije
Copy link
Author

So in case anyone needs this, in the end I went with the following, until I can figure out the types a little better 😕 :

export const graphql = withCustomRequest(
  request.defaults({
    request: {
      async hook(
        req: (options: { headers?: Record<string, string> }) => typeof request,
        options: { headers?: Record<string, string> },
      ) {
        const oAuthToken = await getOAuthToken();

        if (!options.headers) {
          options.headers = {};
        }

        options.headers.authorization = `token ${oAuthToken}`;

        return req(options);
      },
    },
  }),
);

I also tried the following with @octokit/core but couldn't get it to work, and options is not typed (options is of type Function | OctokitOptions which doesn't help tsc), so out-of-luck on that front:

import { Octokit } from '@octokit/core';

const OctokitWithAuth = Octokit.defaults(
  async (options: { headers?: Record<string, string> }) => {
    const oAuthToken = await getOAuthToken();

    return {
      ...options,
      headers: {
        ...options.headers,
        authorization: `token ${oAuthToken}`,
      },
    };
  },
);

const { graphql } = new OctokitWithAuth();

export { graphql };

It seems like the header doesn't get picked up at all, the requests would not get authenticated.

@astorije
Copy link
Author

@gr2m, if you have any suggestions, I'll take 'em 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Support Any questions, information, or general needs around the SDK or GitHub APIs
Projects
None yet
Development

No branches or pull requests

2 participants