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

feat(typescript): do not permit unknown keys on octokit instance #312

Merged
merged 3 commits into from
Apr 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,37 @@ name: Test
types:
- opened
- synchronize

jobs:
test:
test_matrix:
runs-on: ubuntu-latest
strategy:
matrix:
node_version:
- 10
- 12
- 14

steps:
- uses: actions/checkout@v2
- name: "Use Node.js ${{ matrix.node_version }}"
- name: Use Node.js ${{ matrix.node_version }}
uses: actions/setup-node@v2
with:
node-version: "${{ matrix.node_version }}"
node-version: ${{ matrix.node_version }}
- uses: actions/cache@v1
with:
path: ~/.npm
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-node-
- run: npm ci
- run: npm test --ignore-scripts # run lint only once

test:
runs-on: ubuntu-latest
needs: test_matrix
steps:
- uses: actions/checkout@v2
- run: npm ci
- run: npm test
- run: npm run lint
- run: npm run test:typescript
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"lint": "prettier --check '{src,test}/**/*.{ts,md}' README.md package.json",
"lint:fix": "prettier --write '{src,test}/**/*.{ts,md}' README.md package.json",
"pretest": "npm run -s lint",
"test": "jest --coverage"
"test": "jest --coverage",
"test:typescript": "npx tsc --noEmit --declaration --noUnusedLocals test/typescript-validate.ts"
},
"repository": "github:octokit/core.js",
"keywords": [
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class Octokit {
}
};

return OctokitWithDefaults;
return OctokitWithDefaults as typeof this;
}

static plugins: OctokitPlugin[] = [];
Expand All @@ -66,7 +66,7 @@ export class Octokit {
);
};

return NewOctokit as typeof NewOctokit &
return NewOctokit as typeof this &
Constructor<UnionToIntersection<ReturnTypeOf<T>>>;
}

Expand Down
39 changes: 39 additions & 0 deletions test/typescript-validate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// ************************************************************
// THIS CODE IS NOT EXECUTED. IT IS JUST FOR TYPECHECKING
// ************************************************************

import { Octokit } from "../src";

export function pluginsTest() {
// `octokit` instance does not permit unknown keys
const octokit = new Octokit();

// @ts-expect-error Property 'unknown' does not exist on type 'Octokit'.(2339)
octokit.unknown;

const OctokitWithDefaults = Octokit.defaults({});
const octokitWithDefaults = new OctokitWithDefaults();

// Error: `octokitWithDefaults` does permit unknown keys
// @ts-expect-error `.unknown` should not be typed as `any`
octokitWithDefaults.unknown;

const OctokitWithPlugin = Octokit.plugin(() => ({}));
const octokitWithPlugin = new OctokitWithPlugin();

// Error: `octokitWithPlugin` does permit unknown keys
// @ts-expect-error `.unknown` should not be typed as `any`
octokitWithPlugin.unknown;

const OctokitWithPluginAndDefaults = Octokit.plugin(() => ({
foo: 42,
})).defaults({
baz: "daz",
});

const octokitWithPluginAndDefaults = new OctokitWithPluginAndDefaults();

octokitWithPluginAndDefaults.foo;
// @ts-expect-error `.unknown` should not be typed as `any`
octokitWithPluginAndDefaults.unknown;
}