From 6b24e349b277c2fde4da8c0711e969e1467dc9f1 Mon Sep 17 00:00:00 2001 From: Gregor Martynus <39992+gr2m@users.noreply.github.com> Date: Wed, 11 Aug 2021 15:42:25 -0700 Subject: [PATCH 1/2] feat: `@octokit-next/request` --- package-lock.json | 20 ++++++++++++++++- package.json | 16 +++++++++----- packages/core/index.js | 14 +++++++++--- packages/core/package.json | 6 ++--- packages/request/index.d.ts | 3 +++ packages/{core => }/request/index.js | 28 +++++++++++++---------- packages/request/index.test-d.ts | 20 +++++++++++++++++ packages/request/package.json | 22 +++++++++++++++++++ packages/types/index.d.ts | 22 ++++++++++++++----- packages/types/index.test-d.ts | 4 +++- packages/types/request.d.ts | 13 +++++++---- test.js | 33 +++++++++++++++++++++++++--- 12 files changed, 162 insertions(+), 39 deletions(-) create mode 100644 packages/request/index.d.ts rename packages/{core => }/request/index.js (64%) create mode 100644 packages/request/index.test-d.ts create mode 100644 packages/request/package.json diff --git a/package-lock.json b/package-lock.json index c8639b8..c668c7c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,8 @@ "packages/core", "packages/types-rest-api-github.com", "packages/types-rest-api-ghes-3.1", - "packages/types-rest-api-ghes-3.0" + "packages/types-rest-api-ghes-3.0", + "packages/request" ], "dependencies": { "node-fetch": "3.0.0-beta.10" @@ -180,6 +181,10 @@ "resolved": "packages/core", "link": true }, + "node_modules/@octokit-next/request": { + "resolved": "packages/request", + "link": true + }, "node_modules/@octokit-next/types": { "resolved": "packages/types", "link": true @@ -1997,6 +2002,13 @@ "@octokit-next/types": "0.0.0-development" } }, + "packages/request": { + "version": "0.0.0-development", + "license": "MIT", + "dependencies": { + "@octokit-next/types": "0.0.0-development" + } + }, "packages/types": { "name": "@octokit-next/types", "version": "0.0.0-development", @@ -2152,6 +2164,12 @@ "@octokit-next/types": "0.0.0-development" } }, + "@octokit-next/request": { + "version": "file:packages/request", + "requires": { + "@octokit-next/types": "0.0.0-development" + } + }, "@octokit-next/types": { "version": "file:packages/types" }, diff --git a/package.json b/package.json index 3d56773..e6180df 100644 --- a/package.json +++ b/package.json @@ -6,9 +6,6 @@ "author": "Gregor Martynus (https://twitter.com/gr2m)", "license": "MIT", "repository": "github:octokit/octokit-next.js", - "dependencies": { - "node-fetch": "3.0.0-beta.10" - }, "types": "./index.d.ts", "exports": "./index.js", "scripts": { @@ -42,13 +39,19 @@ [ "@semantic-release/npm", { - "pkgRoot": "packages/core" + "pkgRoot": "packages/types" } ], [ "@semantic-release/npm", { - "pkgRoot": "packages/types" + "pkgRoot": "packages/request" + } + ], + [ + "@semantic-release/npm", + { + "pkgRoot": "packages/core" } ], [ @@ -82,7 +85,8 @@ "packages/core", "packages/types-rest-api-github.com", "packages/types-rest-api-ghes-3.1", - "packages/types-rest-api-ghes-3.0" + "packages/types-rest-api-ghes-3.0", + "packages/request" ], "engines": { "node": ">=16.5.0", diff --git a/packages/core/index.js b/packages/core/index.js index 0d8c426..ca41df7 100644 --- a/packages/core/index.js +++ b/packages/core/index.js @@ -1,4 +1,4 @@ -import { request as coreRequest } from "./request/index.js"; +import { request as coreRequest } from "@octokit-next/request"; export class Octokit { static withPlugins(newPlugins) { @@ -35,7 +35,15 @@ export class Octokit { }); } - request(route, parameters) { - return coreRequest(this.options, route, parameters); + request(route, parameters = {}) { + const requestOptions = { + ...this.options.request, + ...parameters.request, + }; + return coreRequest(route, { + ...{ baseUrl: this.options.baseUrl }, + ...parameters, + request: requestOptions, + }); } } diff --git a/packages/core/package.json b/packages/core/package.json index 04babfc..2eb121c 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -8,9 +8,6 @@ "description": "Core SDK module for upcoming Octokit", "exports": "./index.js", "types": "./index.d.ts", - "scripts": { - "test": "tsd" - }, "repository": { "type": "git", "url": "https://github.com/octokit/octokit-next.js.git", @@ -19,6 +16,7 @@ "author": "Gregor Martynus (https://twitter.com/gr2m)", "license": "MIT", "dependencies": { - "@octokit-next/types": "0.0.0-development" + "@octokit-next/types": "0.0.0-development", + "@octokit-next/request": "0.0.0-development" } } diff --git a/packages/request/index.d.ts b/packages/request/index.d.ts new file mode 100644 index 0000000..8bc5637 --- /dev/null +++ b/packages/request/index.d.ts @@ -0,0 +1,3 @@ +import { RequestInterface } from "@octokit-next/types"; + +export declare const request: RequestInterface; diff --git a/packages/core/request/index.js b/packages/request/index.js similarity index 64% rename from packages/core/request/index.js rename to packages/request/index.js index 233479f..3477b0d 100644 --- a/packages/core/request/index.js +++ b/packages/request/index.js @@ -1,5 +1,12 @@ import nodeFetch from "node-fetch"; +const DEFAULTS = { + baseURL: "https://api.github.com", + request: { + fetch: nodeFetch, + }, +}; + /** * Naive implementation of [`@octokit/request`](https://github.com/octokit/request.js/) for testing purposes * @@ -7,28 +14,27 @@ import nodeFetch from "node-fetch"; * @param {string} route * @param {object} [parameters] route parameters */ -export async function request( - { baseUrl, request: { fetch: octokitFetch } }, - route, - parameters = {} -) { +export async function request(route, parameters = {}) { + const requestOptions = { ...DEFAULTS.request, ...parameters.request }; + + const baseUrl = parameters.baseUrl || DEFAULTS.baseURL; const [method, pathOrUrl] = route.split(" "); const url = new URL(pathOrUrl, baseUrl).href; - /* c8 ignore next */ - const fetch = octokitFetch || nodeFetch; - - const response = await fetch(url, { method, headers: parameters.headers }); + const response = await requestOptions.fetch(url, { + method, + headers: parameters.headers, + }); if (!response.ok) { const error = new Error(response.statusText); - error.response = response; error.request = { baseUrl, route, parameters, }; - throw Error; + error.response = response; + throw error; } return { diff --git a/packages/request/index.test-d.ts b/packages/request/index.test-d.ts new file mode 100644 index 0000000..a2e1648 --- /dev/null +++ b/packages/request/index.test-d.ts @@ -0,0 +1,20 @@ +import { expectType } from "tsd"; +import { request } from "./index.js"; + +import "@octokit-next/types-rest-api-ghes-3.1"; + +export async function test() { + const rootEndpointResponse = await request("GET /"); + + expectType(rootEndpointResponse.status); + expectType(rootEndpointResponse.url); + expectType(rootEndpointResponse.headers["x-ratelimit-limit"]); + expectType(rootEndpointResponse.data.emojis_url); + + const ghesOnlyResponse = await request("GET /ghes-only", { + request: { + version: "ghes-3.1", + }, + }); + expectType(ghesOnlyResponse.data.ok); +} diff --git a/packages/request/package.json b/packages/request/package.json new file mode 100644 index 0000000..625531d --- /dev/null +++ b/packages/request/package.json @@ -0,0 +1,22 @@ +{ + "name": "@octokit-next/request", + "version": "0.0.0-development", + "description": "Simplified version of `@octokit/request` to experiment with ESM and types", + "publishConfig": { + "access": "public" + }, + "type": "module", + "exports": "./index.js", + "types": "./index.d.ts", + "repository": { + "type": "git", + "url": "https://github.com/octokit/octokit-next.js.git", + "directory": "packages/core" + }, + "author": "Gregor Martynus (https://twitter.com/gr2m)", + "license": "MIT", + "dependencies": { + "@octokit-next/types": "0.0.0-development", + "node-fetch": "3.0.0-beta.10" + } +} diff --git a/packages/types/index.d.ts b/packages/types/index.d.ts index 21a2d98..fe6a4a7 100644 --- a/packages/types/index.d.ts +++ b/packages/types/index.d.ts @@ -1,10 +1,13 @@ import { RequestInterface } from "./request"; +export { RequestInterface } from "./request"; /** * Global Octokit interfaces that can be extended as needed. */ export namespace Octokit { interface Options { + version?: keyof Octokit.ApiVersions; + /** * GitHub's REST API base URL. Defaults to https://api.github.com * @@ -19,12 +22,19 @@ export namespace Octokit { */ userAgent?: string; - request?: { - /** - * override the built-in fetch method, e.g. for testing - */ - fetch?: (resource: any, init?: any) => Promise<{}>; - }; + request?: RequestOptions; + } + + interface RequestOptions { + /** + * Override API version on a per-request basis. + */ + version?: keyof Octokit.ApiVersions; + + /** + * override the built-in fetch method, e.g. for testing + */ + fetch?: (resource: any, init?: any) => Promise; } interface ResponseHeaders { diff --git a/packages/types/index.test-d.ts b/packages/types/index.test-d.ts index 587508c..1d59ca4 100644 --- a/packages/types/index.test-d.ts +++ b/packages/types/index.test-d.ts @@ -1,6 +1,8 @@ import { expectType } from "tsd"; import { Octokit, Plugin } from "./index.js"; +import "@octokit-next/types-rest-api-github.com"; + export async function test() { // TODO: // new Octokit(); @@ -26,7 +28,7 @@ export async function test() { expectType(emojisResponseDotcom.data["dotcom-only"]); // @ts-expect-error - ghes-only does not exist - emojisResponseDotcom.data["ghes-only"]; + type test = typeof emojisResponseDotcom["data"]["ghes-only"]; const OctokitWithEmptyDefaults = Octokit.withDefaults({ // there should be no required options diff --git a/packages/types/request.d.ts b/packages/types/request.d.ts index 85a6332..4a6e73e 100644 --- a/packages/types/request.d.ts +++ b/packages/types/request.d.ts @@ -1,8 +1,13 @@ import { Octokit } from "./index.js"; -type RequestParameters = Record; +type EndpointParameters = { request: Octokit.RequestOptions } & Record< + string, + unknown +>; -export interface RequestInterface { +export interface RequestInterface< + TVersion extends keyof Octokit.ApiVersions = "github.com" +> { /** * Sends a request based on endpoint options * @@ -14,9 +19,9 @@ export interface RequestInterface { options?: Route extends keyof Octokit.ApiVersions[TVersion]["Endpoints"] ? "parameters" extends keyof Octokit.ApiVersions[TVersion]["Endpoints"][Route] ? Octokit.ApiVersions[TVersion]["Endpoints"][Route]["parameters"] & - RequestParameters + EndpointParameters : never - : RequestParameters + : EndpointParameters ): Route extends keyof Octokit.ApiVersions[TVersion]["Endpoints"] ? "response" extends keyof Octokit.ApiVersions[TVersion]["Endpoints"][Route] ? Promise diff --git a/test.js b/test.js index e94ffb2..bbc4314 100644 --- a/test.js +++ b/test.js @@ -1,6 +1,7 @@ import { test } from "uvu"; import * as assert from "uvu/assert"; +import { request } from "@octokit-next/request"; import { Octokit } from "@octokit-next/core"; test("octokit.request is a function", () => { @@ -18,7 +19,6 @@ test("octokit.request('GET /')", async () => { const fetchMock = async function (url, options) { assert.equal(url, "https://api.github.com/"); assert.equal(options.method, "GET"); - return { status: 200, ok: true, @@ -29,13 +29,11 @@ test("octokit.request('GET /')", async () => { }, }; }; - const octokit = new Octokit({ request: { fetch: fetchMock, }, }); - const response = await octokit.request("GET /"); assert.equal(response, { status: 200, @@ -77,4 +75,33 @@ test("octokit.request('GET /unknown')", async () => { } }); +test("request('GET /')", async () => { + const fetchMock = async function (url, options) { + assert.equal(url, "https://api.github.com/"); + assert.equal(options.method, "GET"); + + return { + status: 200, + ok: true, + headers: new Map(), + url: "https://api.github.com", + async json() { + return { ok: true }; + }, + }; + }; + + const response = await request("GET /", { + request: { + fetch: fetchMock, + }, + }); + assert.equal(response, { + status: 200, + url: "https://api.github.com", + headers: {}, + data: { ok: true }, + }); +}); + test.run(); From e821e02865bbccf40a44497930281696b7c27870 Mon Sep 17 00:00:00 2001 From: Gregor Martynus <39992+gr2m@users.noreply.github.com> Date: Wed, 11 Aug 2021 16:39:38 -0700 Subject: [PATCH 2/2] feat: support setting a per-request version --- packages/types/request.d.ts | 90 ++++++++++++++++++++++++------- tests/js/ghes-3.1/index.test-d.ts | 7 +++ tests/js/request/index.d.ts | 1 + tests/js/request/index.test-d.ts | 29 ++++++++++ tests/ts/request/test.ts | 21 ++++++++ tests/ts/request/tsconfig.json | 4 ++ 6 files changed, 134 insertions(+), 18 deletions(-) create mode 100644 tests/js/request/index.d.ts create mode 100644 tests/js/request/index.test-d.ts create mode 100644 tests/ts/request/test.ts create mode 100644 tests/ts/request/tsconfig.json diff --git a/packages/types/request.d.ts b/packages/types/request.d.ts index 4a6e73e..95421c3 100644 --- a/packages/types/request.d.ts +++ b/packages/types/request.d.ts @@ -5,31 +5,85 @@ type EndpointParameters = { request: Octokit.RequestOptions } & Record< unknown >; +type UnknownResponse = { + /** + * http response headers + */ + headers: Record; + /** + * http response code + */ + status: number; + /** + * URL of response after all redirects + */ + url: string; + /** + * Response data as documented in the REST API reference documentation at https://docs.github.com/rest/reference + */ + data: unknown; +}; + +/** + * The `RequestInterface` is used for both the standalone `@octokit-next/request` module + * as well as `@octokit-next/core`'s `.request()` method. + * + * It has 3 overrides + * + * 1. When passing `{ request: { version }}` as part of the parameters, the passed version + * is used as a base for the types of the remaining parameters and the response + * 2. When a known route is passed, the types for the parameters and the response are + * derived from the version passed in `RequestInterface`, which defaults to `"github.com"` + * 3. When an unknown route is passed, any parameters can be passed, and the response is unknown. + */ export interface RequestInterface< TVersion extends keyof Octokit.ApiVersions = "github.com" > { /** - * Sends a request based on endpoint options + * Send a request to a known endpoint using a version specified in `request.version`. + * + * @param {string} route Request method + URL. Example: `'GET /orgs/{org}'` + * @param {object} parameters URL, query or body parameters, as well as `headers`, `mediaType.{format|previews}`, `request`, or `baseUrl`. + */ + < + RVersion extends keyof Octokit.ApiVersions, + Route extends keyof Octokit.ApiVersions[RVersion]["Endpoints"], + Endpoint = Octokit.ApiVersions[RVersion]["Endpoints"][Route] + >( + route: Route, + options: { + request: { + version: RVersion; + }; + } & ("parameters" extends keyof Endpoint + ? Endpoint["parameters"] & EndpointParameters + : never) + ): "response" extends keyof Endpoint ? Promise : never; + + /** + * Send a request to a known endpoint * * @param {string} route Request method + URL. Example: `'GET /orgs/{org}'` * @param {object} [parameters] URL, query or body parameters, as well as `headers`, `mediaType.{format|previews}`, `request`, or `baseUrl`. */ - ( - route: keyof Octokit.ApiVersions[TVersion]["Endpoints"] | Route, - options?: Route extends keyof Octokit.ApiVersions[TVersion]["Endpoints"] - ? "parameters" extends keyof Octokit.ApiVersions[TVersion]["Endpoints"][Route] - ? Octokit.ApiVersions[TVersion]["Endpoints"][Route]["parameters"] & - EndpointParameters - : never - : EndpointParameters - ): Route extends keyof Octokit.ApiVersions[TVersion]["Endpoints"] - ? "response" extends keyof Octokit.ApiVersions[TVersion]["Endpoints"][Route] - ? Promise + < + Route extends keyof Octokit.ApiVersions[TVersion]["Endpoints"], + Endpoint = Octokit.ApiVersions[TVersion]["Endpoints"][Route] + >( + route: Route, + options?: "parameters" extends keyof Endpoint + ? Endpoint["parameters"] & EndpointParameters : never - : Promise< - Octokit.Response< - unknown, - Octokit.ApiVersions[TVersion]["ResponseHeaders"] - > - >; + ): "response" extends keyof Endpoint ? Promise : never; + + /** + * Send a request to an unknown endpoint + * + * @param {string} route Request method + URL. Example: `'GET /orgs/{org}'` + * @param {object} [parameters] URL, query or body parameters, as well as `headers`, `mediaType.{format|previews}`, `request`, or `baseUrl`. + */ + ( + route: Route, + options?: EndpointParameters + ): Promise; } diff --git a/tests/js/ghes-3.1/index.test-d.ts b/tests/js/ghes-3.1/index.test-d.ts index 526027f..bccc6c4 100644 --- a/tests/js/ghes-3.1/index.test-d.ts +++ b/tests/js/ghes-3.1/index.test-d.ts @@ -15,4 +15,11 @@ export async function test() { const ghesOnlyResponse = await octokit.request("GET /ghes-only"); expectType(ghesOnlyResponse.data.ok); expectType(ghesOnlyResponse.headers["x-dotcom-only"]); + + const dotcomOnlyResponse2 = await octokit.request("GET /dotcom-only", { + request: { + version: "github.com", + }, + }); + expectType(dotcomOnlyResponse2.data.ok); } diff --git a/tests/js/request/index.d.ts b/tests/js/request/index.d.ts new file mode 100644 index 0000000..e65868d --- /dev/null +++ b/tests/js/request/index.d.ts @@ -0,0 +1 @@ +export { request } from "@octokit-next/request"; diff --git a/tests/js/request/index.test-d.ts b/tests/js/request/index.test-d.ts new file mode 100644 index 0000000..fc65b5d --- /dev/null +++ b/tests/js/request/index.test-d.ts @@ -0,0 +1,29 @@ +import { expectType } from "tsd"; +import { request } from "./index.js"; + +import "@octokit-next/types-rest-api-ghes-3.1"; + +export async function test() { + // knownn route, uses explicit version + const ghesOnlyResponse = await request("GET /ghes-only", { + request: { + version: "ghes-3.1", + }, + }); + expectType(ghesOnlyResponse.data.ok); + + // known route, uses "github.com" types by default + const rootEndpointResponse = await request("GET /"); + + expectType(rootEndpointResponse.status); + expectType(rootEndpointResponse.url); + expectType(rootEndpointResponse.headers["x-ratelimit-limit"]); + expectType(rootEndpointResponse.data.emojis_url); + + // unknown route + const unknownResponse = await request("GET /unknown"); + expectType(unknownResponse.status); + expectType(unknownResponse.url); + expectType(unknownResponse.headers["x-ratelimit-limit"]); + expectType(unknownResponse.data); +} diff --git a/tests/ts/request/test.ts b/tests/ts/request/test.ts new file mode 100644 index 0000000..5c836d7 --- /dev/null +++ b/tests/ts/request/test.ts @@ -0,0 +1,21 @@ +import { request } from "@octokit-next/request"; + +import "@octokit-next/types-rest-api-ghes-3.1"; + +function expectType(value: T): void {} + +export async function test() { + const rootEndpointResponse = await request("GET /"); + + expectType(rootEndpointResponse.status); + expectType(rootEndpointResponse.url); + expectType(rootEndpointResponse.headers["x-ratelimit-limit"]); + expectType(rootEndpointResponse.data.emojis_url); + + const ghesOnlyResponse = await request("GET /ghes-only", { + request: { + version: "ghes-3.1", + }, + }); + expectType(ghesOnlyResponse.data.ok); +} diff --git a/tests/ts/request/tsconfig.json b/tests/ts/request/tsconfig.json new file mode 100644 index 0000000..e6f83d5 --- /dev/null +++ b/tests/ts/request/tsconfig.json @@ -0,0 +1,4 @@ +{ + "extends": "../tsconfig.json", + "include": ["*.ts"] +}