From f943bebc1516f3e45a4a96ca6d06e01372e9ee52 Mon Sep 17 00:00:00 2001 From: Sam Seay Date: Tue, 16 May 2023 10:41:37 +1200 Subject: [PATCH] Don't generate singletons for Octokit and GraphQL until they're used at runtime. (#38268) * Move API instance generation into functions to ensure they don't run on import. * Dont use extraneous variables, call singleton fns directly. * Get rid of accidental change --- .../code-freeze/commands/milestone/index.ts | 2 +- .../src/core/github/__tests__/index.ts | 79 ++++++++++--------- tools/monorepo-utils/src/core/github/api.ts | 50 +++++++++--- tools/monorepo-utils/src/core/github/repo.ts | 15 ++-- 4 files changed, 90 insertions(+), 56 deletions(-) diff --git a/tools/monorepo-utils/src/code-freeze/commands/milestone/index.ts b/tools/monorepo-utils/src/code-freeze/commands/milestone/index.ts index ca06f2c284b..936b6d9e4dc 100644 --- a/tools/monorepo-utils/src/code-freeze/commands/milestone/index.ts +++ b/tools/monorepo-utils/src/code-freeze/commands/milestone/index.ts @@ -89,7 +89,7 @@ export const milestoneCommand = new Command( 'milestone' ) } try { - await octokitWithAuth.request( + await octokitWithAuth().request( `POST /repos/${ owner }/${ name }/milestones`, { title: nextMilestone, diff --git a/tools/monorepo-utils/src/core/github/__tests__/index.ts b/tools/monorepo-utils/src/core/github/__tests__/index.ts index 0d47d19a253..1a9f7b3a630 100644 --- a/tools/monorepo-utils/src/core/github/__tests__/index.ts +++ b/tools/monorepo-utils/src/core/github/__tests__/index.ts @@ -5,46 +5,47 @@ import { getLatestGithubReleaseVersion } from '../repo'; jest.mock( '../api', () => { return { - graphqlWithAuth: jest.fn().mockResolvedValue( { - repository: { - releases: { - nodes: [ - { - tagName: 'nightly', - isLatest: false, - }, - { - tagName: 'wc-beta-tester-99.99.0', - isLatest: false, - }, - { - tagName: '1.0.0', - isLatest: false, - }, - { - tagName: '1.1.0', - isLatest: false, - }, - { - tagName: '1.2.0', - isLatest: false, - }, - { - tagName: '2.0.0', - isLatest: false, - }, - { - tagName: '2.0.1', - isLatest: true, - }, - { - tagName: '1.0.1', - isLatest: false, - }, - ], + graphqlWithAuth: () => + jest.fn().mockResolvedValue( { + repository: { + releases: { + nodes: [ + { + tagName: 'nightly', + isLatest: false, + }, + { + tagName: 'wc-beta-tester-99.99.0', + isLatest: false, + }, + { + tagName: '1.0.0', + isLatest: false, + }, + { + tagName: '1.1.0', + isLatest: false, + }, + { + tagName: '1.2.0', + isLatest: false, + }, + { + tagName: '2.0.0', + isLatest: false, + }, + { + tagName: '2.0.1', + isLatest: true, + }, + { + tagName: '1.0.1', + isLatest: false, + }, + ], + }, }, - }, - } ), + } ), }; } ); diff --git a/tools/monorepo-utils/src/core/github/api.ts b/tools/monorepo-utils/src/core/github/api.ts index 8036aadeabd..8a2332cc0ac 100644 --- a/tools/monorepo-utils/src/core/github/api.ts +++ b/tools/monorepo-utils/src/core/github/api.ts @@ -1,20 +1,52 @@ /** * External dependencies */ -import { graphql } from '@octokit/graphql'; +import { graphql as gql } from '@octokit/graphql'; import { Octokit } from 'octokit'; +import { graphql } from '@octokit/graphql/dist-types/types'; /** * Internal dependencies */ import { getEnvVar } from '../environment'; -export const graphqlWithAuth = graphql.defaults( { - headers: { - authorization: `Bearer ${ getEnvVar( 'GITHUB_TOKEN', true ) }`, - }, -} ); +let graphqlWithAuthInstance; +let octokitWithAuthInstance; -export const octokitWithAuth = new Octokit( { - auth: getEnvVar( 'GITHUB_TOKEN', true ), -} ); +/** + * Returns a graphql instance with auth headers, throws an Exception if + * `GITHUB_TOKEN` env var is not present. + * + * @return graphql instance + */ +export const graphqlWithAuth = (): graphql => { + if ( graphqlWithAuthInstance ) { + return graphqlWithAuthInstance; + } + + graphqlWithAuthInstance = gql.defaults( { + headers: { + authorization: `Bearer ${ getEnvVar( 'GITHUB_TOKEN', true ) }`, + }, + } ); + + return graphqlWithAuthInstance; +}; + +/** + * Returns an Octokit instance with auth headers, throws an Exception if + * `GITHUB_TOKEN` env var is not present. + * + * @return graphql instance + */ +export const octokitWithAuth = (): Octokit => { + if ( octokitWithAuthInstance ) { + return octokitWithAuthInstance; + } + + octokitWithAuthInstance = new Octokit( { + auth: getEnvVar( 'GITHUB_TOKEN', true ), + } ); + + return octokitWithAuthInstance; +}; diff --git a/tools/monorepo-utils/src/core/github/repo.ts b/tools/monorepo-utils/src/core/github/repo.ts index 572351aefcd..ba7a77813fe 100644 --- a/tools/monorepo-utils/src/core/github/repo.ts +++ b/tools/monorepo-utils/src/core/github/repo.ts @@ -7,7 +7,6 @@ import { Repository } from '@octokit/graphql-schema'; * Internal dependencies */ import { graphqlWithAuth, octokitWithAuth } from './api'; -import { Logger } from '../logger'; import { PullRequestEndpointResponse } from './types'; export const getLatestGithubReleaseVersion = async ( options: { @@ -16,7 +15,7 @@ export const getLatestGithubReleaseVersion = async ( options: { } ): Promise< string > => { const { owner, name } = options; - const data = await graphqlWithAuth< { repository: Repository } >( ` + const data = await graphqlWithAuth()< { repository: Repository } >( ` { repository(owner: "${ owner }", name: "${ name }") { releases( @@ -45,8 +44,9 @@ export const doesGithubBranchExist = async ( nextReleaseBranch: string ): Promise< boolean > => { const { owner, name } = options; + try { - const branchOnGithub = await octokitWithAuth.request( + const branchOnGithub = await octokitWithAuth().request( 'GET /repos/{owner}/{repo}/branches/{branch}', { owner, @@ -74,7 +74,7 @@ export const getRefFromGithubBranch = async ( source: string ): Promise< string > => { const { owner, name } = options; - const { repository } = await graphqlWithAuth< { + const { repository } = await graphqlWithAuth()< { repository: Repository; } >( ` { @@ -105,7 +105,7 @@ export const createGithubBranch = async ( ref: string ): Promise< void > => { const { owner, name } = options; - await octokitWithAuth.request( 'POST /repos/{owner}/{repo}/git/refs', { + await octokitWithAuth().request( 'POST /repos/{owner}/{repo}/git/refs', { owner, repo: name, ref: `refs/heads/${ branch }`, @@ -121,7 +121,7 @@ export const deleteGithubBranch = async ( branch: string ): Promise< void > => { const { owner, name } = options; - await octokitWithAuth.request( + await octokitWithAuth().request( 'DELETE /repos/{owner}/{repo}/git/refs/heads/{ref}', { owner, @@ -152,7 +152,7 @@ export const createPullRequest = async ( options: { body: string; } ): Promise< PullRequestEndpointResponse[ 'data' ] > => { const { head, base, owner, name, title, body } = options; - const pullRequest = await octokitWithAuth.request( + const pullRequest = await octokitWithAuth().request( 'POST /repos/{owner}/{repo}/pulls', { owner, @@ -163,6 +163,7 @@ export const createPullRequest = async ( options: { base, } ); + //@ts-ignore There is a type mismatch between the graphql schema and the response. pullRequest.data.head.repo.has_discussions is a boolean, but the graphql schema doesn't have that field. return pullRequest.data; };