From 7347cdd9e51d01506a32e3ce46afa860c5aa778a Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Tue, 7 Jan 2020 13:17:25 -0500 Subject: [PATCH 1/8] Add Webhooks package definitions --- packages/github/__tests__/lib.test.ts | 2 +- packages/github/package-lock.json | 23 ++++++++++ packages/github/package.json | 3 +- packages/github/src/context.ts | 5 +-- packages/github/src/interfaces.ts | 63 ++++++++++++--------------- 5 files changed, 55 insertions(+), 41 deletions(-) diff --git a/packages/github/__tests__/lib.test.ts b/packages/github/__tests__/lib.test.ts index 5c042f2a8c..3ceb361d8b 100644 --- a/packages/github/__tests__/lib.test.ts +++ b/packages/github/__tests__/lib.test.ts @@ -55,7 +55,7 @@ describe('@actions/context', () => { it('works with pullRequest payloads', () => { delete process.env.GITHUB_REPOSITORY context.payload = { - pullRequest: {number: 2}, + pull_request: {number: 2}, repository: {owner: {login: 'user'}, name: 'test'} } expect(context.issue).toEqual({ diff --git a/packages/github/package-lock.json b/packages/github/package-lock.json index 6dcc35ef9f..d891b320e2 100644 --- a/packages/github/package-lock.json +++ b/packages/github/package-lock.json @@ -524,6 +524,29 @@ "@types/node": ">= 8" } }, + "@octokit/webhooks": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/@octokit/webhooks/-/webhooks-7.0.0.tgz", + "integrity": "sha512-oSZuKc2LDNtt3vW7Iq9XNvIm3i6CxMkrzkuyHinR6IjVRb7EuiMshn3AVDdXdDtAVHvxwxj3ikt3jsTlFE6zEA==", + "requires": { + "debug": "^4.0.0" + }, + "dependencies": { + "debug": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", + "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", + "requires": { + "ms": "^2.1.1" + } + }, + "ms": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", + "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" + } + } + }, "@types/babel__core": { "version": "7.1.2", "resolved": "https://registry.npmjs.org/@types/babel__core/-/babel__core-7.1.2.tgz", diff --git a/packages/github/package.json b/packages/github/package.json index b956d42363..6d3ccbdf56 100644 --- a/packages/github/package.json +++ b/packages/github/package.json @@ -38,7 +38,8 @@ }, "dependencies": { "@octokit/graphql": "^4.3.1", - "@octokit/rest": "^16.15.0" + "@octokit/rest": "^16.15.0", + "@octokit/webhooks": "^7.0.0" }, "devDependencies": { "jest": "^24.7.1" diff --git a/packages/github/src/context.ts b/packages/github/src/context.ts index f2aafde112..713ccfbcff 100644 --- a/packages/github/src/context.ts +++ b/packages/github/src/context.ts @@ -1,5 +1,5 @@ // Originally pulled from https://github.com/JasonEtco/actions-toolkit/blob/master/src/context.ts -import {WebhookPayload} from './interfaces' +import {WebhookPayload, Webhooks} from './interfaces' import {readFileSync, existsSync} from 'fs' import {EOL} from 'os' @@ -41,10 +41,9 @@ export class Context { get issue(): {owner: string; repo: string; number: number} { const payload = this.payload - return { ...this.repo, - number: (payload.issue || payload.pullRequest || payload).number + number: (payload.issue || payload.pull_request || payload).number } } diff --git a/packages/github/src/interfaces.ts b/packages/github/src/interfaces.ts index c20aadd827..7de60a89f8 100644 --- a/packages/github/src/interfaces.ts +++ b/packages/github/src/interfaces.ts @@ -1,39 +1,30 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -export interface PayloadRepository { - [key: string]: any - full_name?: string - name: string - owner: { - [key: string]: any - login: string - name?: string - } - html_url?: string -} +import Webhooks from '@octokit/webhooks' +type WebhookPayload = + | Webhooks.WebhookPayloadPush + | Webhooks.WebhookPayloadPullRequest + | Webhooks.WebhookPayloadPullRequestReview + | Webhooks.WebhookPayloadPullRequestReviewComment + | Webhooks.WebhookPayloadStatus + | Webhooks.WebhookPayloadIssues + | Webhooks.WebhookPayloadIssueComment + | Webhooks.WebhookPayloadRelease + | Webhooks.WebhookPayloadRepositoryDispatch + | Webhooks.WebhookPayloadCheckRun + | Webhooks.WebhookPayloadDeployment + | Webhooks.WebhookPayloadCheckSuite + | Webhooks.WebhookPayloadWatch + | Webhooks.WebhookPayloadDeploymentStatus + | Webhooks.WebhookPayloadCreate + | Webhooks.WebhookPayloadDelete + | Webhooks.WebhookPayloadProjectCard + | Webhooks.WebhookPayloadPageBuild + | Webhooks.WebhookPayloadFork + | Webhooks.WebhookPayloadGollum + | Webhooks.WebhookPayloadMilestone + | Webhooks.WebhookPayloadProject + | Webhooks.WebhookPayloadLabel + | any -export interface WebhookPayload { - [key: string]: any - repository?: PayloadRepository - issue?: { - [key: string]: any - number: number - html_url?: string - body?: string - } - pull_request?: { - [key: string]: any - number: number - html_url?: string - body?: string - } - sender?: { - [key: string]: any - type: string - } - action?: string - installation?: { - id: number - [key: string]: any - } -} +export {Webhooks, WebhookPayload} From 99fffe8b48ed1404decaff4390dcf5544ef702a8 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Wed, 8 Jan 2020 14:25:34 -0500 Subject: [PATCH 2/8] Make payload optional, remove any typing --- packages/github/README.md | 26 +++++++++++++++++++++++ packages/github/__tests__/lib.test.ts | 30 +++++++++++++++------------ packages/github/src/context.ts | 11 +++++----- packages/github/src/interfaces.ts | 1 - 4 files changed, 48 insertions(+), 20 deletions(-) diff --git a/packages/github/README.md b/packages/github/README.md index 12c5b0b078..70f934879a 100644 --- a/packages/github/README.md +++ b/packages/github/README.md @@ -55,3 +55,29 @@ const newIssue = await octokit.issues.create({ body: 'Hello Universe!' }); ``` + +### Context Payload Casting +The [GitHub Webhook Event](https://developer.github.com/webhooks/#events) payload is provided as a part of the context. Based on the type of event that [triggered your workflow](https://help.github.com/en/actions/automating-your-workflow-with-github-actions/events-that-trigger-workflows), the structure of that object may change. You can cast this field to access these members in a type safe way. + +```js +const github = require('@actions/github'); + +const payload = github.context.payload +if ((payload as Webhooks.WebhookPayloadPush).head_commit) { + core.info( + `The head commit is: ${ + (payload as Webhooks.WebhookPayloadPush).head_commit + }` + ) +} +``` + +There may be cases where multiple events have a field you need. For example, we can get the issue number field from the `issue` event and the `issue_comment` event +```js +const github = require('@actions/github'); +const payload = github.context.payload +if ("issue" in payload) { + core.info(`The issue number is: ${JSON.stringify(payload.issue.number)}`) +} +``` + diff --git a/packages/github/__tests__/lib.test.ts b/packages/github/__tests__/lib.test.ts index 3ceb361d8b..21c1556596 100644 --- a/packages/github/__tests__/lib.test.ts +++ b/packages/github/__tests__/lib.test.ts @@ -1,5 +1,7 @@ import * as path from 'path' import {Context} from '../src/context' +import { PayloadRepository } from '@octokit/webhooks' +import { WebhookPayload } from '../src/interfaces' /* eslint-disable @typescript-eslint/no-require-imports */ @@ -16,11 +18,18 @@ describe('@actions/context', () => { expect(context.payload).toEqual(require('./payload.json')) }) - it('returns an empty payload if the GITHUB_EVENT_PATH environment variable is falsey', () => { + it('returns an undefined payload if the GITHUB_EVENT_PATH environment variable is falsey', () => { delete process.env.GITHUB_EVENT_PATH context = new Context() - expect(context.payload).toEqual({}) + expect(context.payload).toEqual(undefined) + }) + + it('returns an undefined payload if the GITHUB_EVENT_PATH environment variable does not point to a file', () => { + process.env.GITHUB_EVENT_PATH = path.join(__dirname, 'this_is_a_fake_file.json') + + context = new Context() + expect(context.payload).toEqual(undefined) }) it('returns attributes from the GITHUB_REPOSITORY', () => { @@ -29,18 +38,13 @@ describe('@actions/context', () => { it('returns attributes from the repository payload', () => { delete process.env.GITHUB_REPOSITORY - - context.payload.repository = { - name: 'test', - owner: {login: 'user'} - } expect(context.repo).toEqual({owner: 'user', repo: 'test'}) }) it("return error for context.repo when repository doesn't exist", () => { delete process.env.GITHUB_REPOSITORY - - context.payload.repository = undefined + delete process.env.GITHUB_EVENT_PATH + context = new Context() expect(() => context.repo).toThrowErrorMatchingSnapshot() }) @@ -56,8 +60,8 @@ describe('@actions/context', () => { delete process.env.GITHUB_REPOSITORY context.payload = { pull_request: {number: 2}, - repository: {owner: {login: 'user'}, name: 'test'} - } + repository: {owner: {login: 'user'}, name: 'test'} as PayloadRepository + } as WebhookPayload expect(context.issue).toEqual({ number: 2, owner: 'user', @@ -69,8 +73,8 @@ describe('@actions/context', () => { delete process.env.GITHUB_REPOSITORY context.payload = { number: 2, - repository: {owner: {login: 'user'}, name: 'test'} - } + repository: {owner: {login: 'user'}, name: 'test'} as PayloadRepository + } as WebhookPayload expect(context.issue).toEqual({ number: 2, owner: 'user', diff --git a/packages/github/src/context.ts b/packages/github/src/context.ts index 713ccfbcff..a064e5da4f 100644 --- a/packages/github/src/context.ts +++ b/packages/github/src/context.ts @@ -7,7 +7,7 @@ export class Context { /** * Webhook payload object that triggered the workflow */ - payload: WebhookPayload + payload?: WebhookPayload eventName: string sha: string @@ -20,7 +20,6 @@ export class Context { * Hydrate the context from the environment */ constructor() { - this.payload = {} if (process.env.GITHUB_EVENT_PATH) { if (existsSync(process.env.GITHUB_EVENT_PATH)) { this.payload = JSON.parse( @@ -40,7 +39,7 @@ export class Context { } get issue(): {owner: string; repo: string; number: number} { - const payload = this.payload + const payload = this.payload as any; return { ...this.repo, number: (payload.issue || payload.pull_request || payload).number @@ -53,10 +52,10 @@ export class Context { return {owner, repo} } - if (this.payload.repository) { + if (this.payload?.repository) { return { - owner: this.payload.repository.owner.login, - repo: this.payload.repository.name + owner: this.payload?.repository.owner.login, + repo: this.payload?.repository.name } } diff --git a/packages/github/src/interfaces.ts b/packages/github/src/interfaces.ts index 7de60a89f8..0862e0cd61 100644 --- a/packages/github/src/interfaces.ts +++ b/packages/github/src/interfaces.ts @@ -25,6 +25,5 @@ type WebhookPayload = | Webhooks.WebhookPayloadMilestone | Webhooks.WebhookPayloadProject | Webhooks.WebhookPayloadLabel - | any export {Webhooks, WebhookPayload} From e6c3ae7c3a0d2c0066a9a93b31febd3e2ee08d36 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Wed, 8 Jan 2020 14:29:29 -0500 Subject: [PATCH 3/8] Fix code in docs --- packages/github/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/github/README.md b/packages/github/README.md index 70f934879a..f055f004a1 100644 --- a/packages/github/README.md +++ b/packages/github/README.md @@ -76,7 +76,7 @@ There may be cases where multiple events have a field you need. For example, we ```js const github = require('@actions/github'); const payload = github.context.payload -if ("issue" in payload) { +if (payload && "issue" in payload) { core.info(`The issue number is: ${JSON.stringify(payload.issue.number)}`) } ``` From 730170e5a0d12f8f8c1b2606f2d6dff981bb06e6 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Wed, 8 Jan 2020 15:30:08 -0500 Subject: [PATCH 4/8] Fix Linting issues --- packages/github/__tests__/lib.test.ts | 7 ++++--- packages/github/src/context.ts | 5 +++-- packages/github/src/interfaces.ts | 2 -- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/github/__tests__/lib.test.ts b/packages/github/__tests__/lib.test.ts index 21c1556596..9fbdd2b1f3 100644 --- a/packages/github/__tests__/lib.test.ts +++ b/packages/github/__tests__/lib.test.ts @@ -1,7 +1,7 @@ import * as path from 'path' import {Context} from '../src/context' -import { PayloadRepository } from '@octokit/webhooks' -import { WebhookPayload } from '../src/interfaces' +import {PayloadRepository} from '@octokit/webhooks' +import {WebhookPayload} from '../src/interfaces' /* eslint-disable @typescript-eslint/no-require-imports */ @@ -26,7 +26,7 @@ describe('@actions/context', () => { }) it('returns an undefined payload if the GITHUB_EVENT_PATH environment variable does not point to a file', () => { - process.env.GITHUB_EVENT_PATH = path.join(__dirname, 'this_is_a_fake_file.json') + process.env.GITHUB_EVENT_PATH = path.join(__dirname, 'invalidfile.json') context = new Context() expect(context.payload).toEqual(undefined) @@ -59,6 +59,7 @@ describe('@actions/context', () => { it('works with pullRequest payloads', () => { delete process.env.GITHUB_REPOSITORY context.payload = { + // eslint-disable-next-line @typescript-eslint/camelcase pull_request: {number: 2}, repository: {owner: {login: 'user'}, name: 'test'} as PayloadRepository } as WebhookPayload diff --git a/packages/github/src/context.ts b/packages/github/src/context.ts index a064e5da4f..c6d30634ec 100644 --- a/packages/github/src/context.ts +++ b/packages/github/src/context.ts @@ -1,5 +1,5 @@ // Originally pulled from https://github.com/JasonEtco/actions-toolkit/blob/master/src/context.ts -import {WebhookPayload, Webhooks} from './interfaces' +import {WebhookPayload} from './interfaces' import {readFileSync, existsSync} from 'fs' import {EOL} from 'os' @@ -39,7 +39,8 @@ export class Context { } get issue(): {owner: string; repo: string; number: number} { - const payload = this.payload as any; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const payload = this.payload as any return { ...this.repo, number: (payload.issue || payload.pull_request || payload).number diff --git a/packages/github/src/interfaces.ts b/packages/github/src/interfaces.ts index 0862e0cd61..ef6260014d 100644 --- a/packages/github/src/interfaces.ts +++ b/packages/github/src/interfaces.ts @@ -1,5 +1,3 @@ -/* eslint-disable @typescript-eslint/no-explicit-any */ - import Webhooks from '@octokit/webhooks' type WebhookPayload = | Webhooks.WebhookPayloadPush From 1864dee8c77b8f096fa661aa55ffd1f069148da4 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Wed, 8 Jan 2020 16:17:02 -0500 Subject: [PATCH 5/8] Address Linter Warnings --- packages/github/__tests__/lib.test.ts | 3 +++ packages/github/src/context.ts | 3 ++- packages/github/src/interfaces.ts | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/github/__tests__/lib.test.ts b/packages/github/__tests__/lib.test.ts index 9fbdd2b1f3..459317eaee 100644 --- a/packages/github/__tests__/lib.test.ts +++ b/packages/github/__tests__/lib.test.ts @@ -5,6 +5,9 @@ import {WebhookPayload} from '../src/interfaces' /* eslint-disable @typescript-eslint/no-require-imports */ +// TODO: https://github.com/actions/toolkit/issues/291, ESLint chokes on the a?.b syntax introduced in Typescript 3.7 +/* eslint-disable @typescript-eslint/no-object-literal-type-assertion */ + describe('@actions/context', () => { let context: Context diff --git a/packages/github/src/context.ts b/packages/github/src/context.ts index c6d30634ec..3884244954 100644 --- a/packages/github/src/context.ts +++ b/packages/github/src/context.ts @@ -39,7 +39,8 @@ export class Context { } get issue(): {owner: string; repo: string; number: number} { - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // TODO: https://github.com/actions/toolkit/issues/291 to remove no-unnecessary-type-assertion + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unnecessary-type-assertion const payload = this.payload as any return { ...this.repo, diff --git a/packages/github/src/interfaces.ts b/packages/github/src/interfaces.ts index ef6260014d..4869b6c79e 100644 --- a/packages/github/src/interfaces.ts +++ b/packages/github/src/interfaces.ts @@ -1,3 +1,5 @@ +// TODO: https://github.com/actions/toolkit/issues/291, ESLint chokes on the | syntax +/* eslint-disable no-undef */ import Webhooks from '@octokit/webhooks' type WebhookPayload = | Webhooks.WebhookPayloadPush From c28f7df71b87afd178d9842cf1c502c4d8a90574 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Wed, 8 Jan 2020 16:37:15 -0500 Subject: [PATCH 6/8] Readme readability updates --- packages/github/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/github/README.md b/packages/github/README.md index f055f004a1..c4a6a9ed88 100644 --- a/packages/github/README.md +++ b/packages/github/README.md @@ -56,10 +56,10 @@ const newIssue = await octokit.issues.create({ }); ``` -### Context Payload Casting +### Context Payload Casting in Typescript The [GitHub Webhook Event](https://developer.github.com/webhooks/#events) payload is provided as a part of the context. Based on the type of event that [triggered your workflow](https://help.github.com/en/actions/automating-your-workflow-with-github-actions/events-that-trigger-workflows), the structure of that object may change. You can cast this field to access these members in a type safe way. -```js +```ts const github = require('@actions/github'); const payload = github.context.payload @@ -73,11 +73,11 @@ if ((payload as Webhooks.WebhookPayloadPush).head_commit) { ``` There may be cases where multiple events have a field you need. For example, we can get the issue number field from the `issue` event and the `issue_comment` event -```js +```ts const github = require('@actions/github'); const payload = github.context.payload if (payload && "issue" in payload) { - core.info(`The issue number is: ${JSON.stringify(payload.issue.number)}`) + core.info(`The issue number is: ${payload.issue.number}`) } ``` From f3b1b7b76eae079fc36c5f1cc3d10297531ec74e Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Mon, 13 Jan 2020 16:47:35 -0500 Subject: [PATCH 7/8] Update readme and exports --- packages/github/README.md | 22 ++++++++++++---------- packages/github/src/interfaces.ts | 4 +--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/github/README.md b/packages/github/README.md index c4a6a9ed88..ba8224ac2f 100644 --- a/packages/github/README.md +++ b/packages/github/README.md @@ -60,24 +60,26 @@ const newIssue = await octokit.issues.create({ The [GitHub Webhook Event](https://developer.github.com/webhooks/#events) payload is provided as a part of the context. Based on the type of event that [triggered your workflow](https://help.github.com/en/actions/automating-your-workflow-with-github-actions/events-that-trigger-workflows), the structure of that object may change. You can cast this field to access these members in a type safe way. ```ts -const github = require('@actions/github'); +import * as core from '@actions/core' +import * as github from '@actions/github' +import * as Webhooks from '@octokit/webhooks' const payload = github.context.payload -if ((payload as Webhooks.WebhookPayloadPush).head_commit) { - core.info( - `The head commit is: ${ - (payload as Webhooks.WebhookPayloadPush).head_commit - }` - ) +if (github.context.eventName === 'push') { + const pushPayload = github.context.payload as Webhooks.WebhookPayloadPush + core.info(`The head commit is: ${pushPayload.head_commit}`) } ``` There may be cases where multiple events have a field you need. For example, we can get the issue number field from the `issue` event and the `issue_comment` event ```ts -const github = require('@actions/github'); +import * as core from '@actions/core' +import * as github from '@actions/github' +import * as Webhooks from '@octokit/webhooks' + const payload = github.context.payload -if (payload && "issue" in payload) { - core.info(`The issue number is: ${payload.issue.number}`) +if (payload && 'pull_request' in payload) { + core.info(`The issue is: ${JSON.stringify(payload.pull_request.number)}`) } ``` diff --git a/packages/github/src/interfaces.ts b/packages/github/src/interfaces.ts index 4869b6c79e..a51ce16202 100644 --- a/packages/github/src/interfaces.ts +++ b/packages/github/src/interfaces.ts @@ -1,7 +1,7 @@ // TODO: https://github.com/actions/toolkit/issues/291, ESLint chokes on the | syntax /* eslint-disable no-undef */ import Webhooks from '@octokit/webhooks' -type WebhookPayload = +export type WebhookPayload = | Webhooks.WebhookPayloadPush | Webhooks.WebhookPayloadPullRequest | Webhooks.WebhookPayloadPullRequestReview @@ -25,5 +25,3 @@ type WebhookPayload = | Webhooks.WebhookPayloadMilestone | Webhooks.WebhookPayloadProject | Webhooks.WebhookPayloadLabel - -export {Webhooks, WebhookPayload} From 09ed431f949d0335efe62ce2db70150ced2bfdfe Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Tue, 14 Jan 2020 12:48:03 -0500 Subject: [PATCH 8/8] remove unneeded lint exception --- packages/github/src/interfaces.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/github/src/interfaces.ts b/packages/github/src/interfaces.ts index a51ce16202..7f2f367eb3 100644 --- a/packages/github/src/interfaces.ts +++ b/packages/github/src/interfaces.ts @@ -1,5 +1,3 @@ -// TODO: https://github.com/actions/toolkit/issues/291, ESLint chokes on the | syntax -/* eslint-disable no-undef */ import Webhooks from '@octokit/webhooks' export type WebhookPayload = | Webhooks.WebhookPayloadPush