Skip to content

Commit 8f5adb3

Browse files
authored
Manual type declarations in openapi-fetch (#1424)
* Separate TS types to be managed manually * Update CONTRIBUTING.md * Cleanup
1 parent bebf08c commit 8f5adb3

19 files changed

+1171
-623
lines changed

.changeset/fair-carpets-beg.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"openapi-fetch": patch
3+
---
4+
5+
Separate TS types to be managed manually

packages/openapi-fetch/.eslintignore

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
test/openapi-typescript-codegen.min.js
1+
test/fixtures

packages/openapi-fetch/CONTRIBUTING.md

+48-28
Original file line numberDiff line numberDiff line change
@@ -10,45 +10,23 @@ Please check out the [the open issues](https://github.com/drwpow/openapi-typescr
1010

1111
Contributing doesn’t have to be in code. Simply answering questions in open issues or providing workarounds is as important as making pull requests.
1212

13-
## Opening a Pull Request
14-
15-
Pull requests are **welcome** for this repo!
16-
17-
Bugfixes will always be accepted, though in some cases some small changes may be requested.
18-
19-
However, if adding a feature or breaking change, please **open an issue first to discuss.** This ensures no time or work is wasted writing code that won’t be accepted to the project (see [Project Goals](https://openapi-ts.pages.dev/openapi-fetch/about/#project-goals)). Undiscussed feature work may be rejected at the discretion of the maintainers.
13+
## Writing code
2014

2115
### Setup
2216

2317
1. Install [pnpm](https://pnpm.io/)
2418
2. [Fork this repo](https://docs.github.com/en/get-started/quickstart/fork-a-repo) and clone your copy locally
2519
3. Run `pnpm i` to install dependencies
2620

27-
### Writing code
28-
29-
Create a new branch for your PR with `git checkout -b your-branch-name`. Add the relevant code as well as docs and tests. When you push everything up (`git push`), navigate back to your repo in GitHub and you should see a prompt to open a new PR.
30-
31-
While best practices for commit messages are encouraged (e.g. start with an imperative verb, keep it short, use the body if needed), this repo doesn’t follow any specific guidelines. Clarity is favored over strict rules. Changelogs are generated separately from git (see [the Changelogs section](#changelogs))
32-
33-
### Writing the PR
34-
35-
**Please fill out the template!** It’s a very lightweight template 🙂.
36-
37-
### Changelogs
38-
39-
The changelog is generated via [changesets](https://github.com/changesets/changesets), and is separate from Git commit messages and pull request titles. To write a human-readable changelog for your changes, run:
40-
41-
```
42-
npx changeset
43-
```
21+
### Runtime code vs static type declarations
4422

45-
This will ask if it’s a `patch`, `minor`, or `major` change ([semver](https://semver.org/)), along with a plain description of what you did. Commit this new file along with the rest of your PR, and during the next release this will go into the official changelog!
23+
This project uses **manual type declarations** ([docs](https://www.typescriptlang.org/docs/handbook/2/type-declarations.html#dts-files)) that are kept separately from the runtime code. `index.d.ts` contains **type declarations**, and `index.js` contains **runtime code**. The most important code lives in `index.d.ts` because this library exists to provide correct type inference. `index.js`, is far less important, and is only doing bare-minimum work to support the API as efficiently as possible.
4624

47-
### CI
25+
In most projects, **this is not recommended practice** as the two can (and will) diverge, and you’re left with types that “lie” to you about what the runtime is doing. So this project does have that liability. However, due to the unique nature of this project implementing complex type inference from user-provided schemas, the type inference itself is so complex it was interfering with readable, maintainable, runtime code. By separating the two we won’t have the more complex parts of TypeScript interfering with the optimal runtime code. This would not be tenable in a larger project, but is perfect for a small codebase like this with minimal surface area.
4826

49-
All PRs must fix lint errors, and all tests must pass. PRs will not be merged until all CI checks are “green” (✅).
27+
When writing code, it’s tempting to ignore `index.d.ts` and only make runtime updates, since that is generally simpler. But **please don’t!** We write tests in `*.test.ts` files intentionally so that the type inference can be typechecked as well as the runtime. Whenever you make a change to `index.js`, you probably need to also make a chance to `index.d.ts`, too. And **testing type correctness in tests is just as important as testing the runtime!**
5028

51-
#### Tests
29+
### Testing
5230

5331
This library uses [Vitest](https://vitest.dev/) for testing. There’s a great [VS Code extension](https://marketplace.visualstudio.com/items?itemName=ZixuanChen.vitest-explorer) you can optionally use if you’d like in-editor debugging tools.
5432

@@ -69,3 +47,45 @@ To start the entire test suite in watch mode:
6947
```bash
7048
npx vitest
7149
```
50+
51+
#### TypeScript tests
52+
53+
**Don’t neglect writing TS tests!** In the test suite, you’ll see `// @ts-expect-error` comments. These are critical tests in and of themselves—they are asserting that TypeScript throws an error when it should be throwing an error (the test suite will actually fail in places if a TS error is _not_ raised).
54+
55+
As this is just a minimal fetch wrapper meant to provide deep type inference for API schemas, **testing TS types** is arguably more important than testing the runtime. So please make liberal use of `// @ts-expect-error`, and as a general rule of thumb, write more **unwanted** output tests than _wanted_ output tests.
56+
57+
### Changelogs
58+
59+
The changelog is generated via [changesets](https://github.com/changesets/changesets), and is separate from Git commit messages and pull request titles. To write a human-readable changelog for your changes, run:
60+
61+
```
62+
npx changeset
63+
```
64+
65+
This will ask if it’s a `patch`, `minor`, or `major` change ([semver](https://semver.org/)), along with a plain description of what you did. Commit this new file along with the rest of your PR, and during the next release this will go into the official changelog!
66+
67+
## Opening a Pull Request
68+
69+
Pull requests are **welcome** for this repo!
70+
71+
Bugfixes will always be accepted, though in some cases some small changes may be requested.
72+
73+
However, if adding a feature or breaking change, please **open an issue first to discuss.** This ensures no time or work is wasted writing code that won’t be accepted to the project (see [Project Goals](https://openapi-ts.pages.dev/openapi-fetch/about/#project-goals)). Undiscussed feature work may be rejected at the discretion of the maintainers.
74+
75+
### Writing the commit
76+
77+
Create a new branch for your PR with `git checkout -b your-branch-name`. Add the relevant code as well as docs and tests. When you push everything up (`git push`), navigate back to your repo in GitHub and you should see a prompt to open a new PR.
78+
79+
While best practices for commit messages are encouraged (e.g. start with an imperative verb, keep it short, use the body if needed), this repo doesn’t follow any specific guidelines. Clarity is favored over strict rules. Changelogs are generated separately from git (see [the Changelogs section](#changelogs)).
80+
81+
### Writing the PR notes
82+
83+
**Please fill out the template!** It’s a very lightweight template 🙂.
84+
85+
### Adding docs
86+
87+
If you added a feature, or changed how something worked, please [update the docs](../../docs/)!
88+
89+
### Passing CI
90+
91+
All PRs must fix lint errors, and all tests must pass. PRs will not be merged until all CI checks are “green” (✅).

packages/openapi-fetch/package.json

+8-8
Original file line numberDiff line numberDiff line change
@@ -47,29 +47,29 @@
4747
"svelte"
4848
],
4949
"scripts": {
50-
"build": "pnpm run build:clean && pnpm run build:ts && pnpm run build:ts-min && pnpm run build:cjs",
50+
"build": "pnpm run build:clean && pnpm run build:js && pnpm run build:js-min && pnpm run build:cjs",
5151
"build:clean": "del dist",
52-
"build:ts": "tsc -p tsconfig.build.json",
53-
"build:ts-min": "esbuild --bundle src/index.ts --format=esm --minify --outfile=dist/index.min.js && cp dist/index.d.ts dist/index.min.d.ts",
54-
"build:cjs": "esbuild --bundle src/index.ts --format=cjs --outfile=dist/cjs/index.cjs && cp dist/index.d.ts dist/cjs/index.d.cts",
52+
"build:js": "mkdir -p dist && cp src/* dist",
53+
"build:js-min": "esbuild --bundle src/index.js --format=esm --minify --outfile=dist/index.min.js && cp dist/index.d.ts dist/index.min.d.ts",
54+
"build:cjs": "esbuild --bundle src/index.js --format=cjs --outfile=dist/cjs/index.cjs && cp dist/index.d.ts dist/cjs/index.d.cts",
5555
"lint": "pnpm run lint:js",
5656
"lint:js": "eslint \"{src,test}/**/*.{js,ts}\"",
5757
"lint:prettier": "prettier --check \"{src,test}/**/*\"",
5858
"test": "pnpm run test:ts && npm run test:js",
5959
"test:js": "vitest run",
6060
"test:ts": "tsc --noEmit",
61-
"prepare": "openapi-typescript test/v1.yaml -o test/v1.d.ts",
61+
"prepare": "openapi-typescript test/fixtures/api.yaml -o test/fixtures/api.d.ts",
6262
"prepublish": "pnpm run prepare && pnpm run build",
6363
"version": "pnpm run prepare && pnpm run build"
6464
},
6565
"dependencies": {
6666
"openapi-typescript-helpers": "^0.0.4"
6767
},
6868
"devDependencies": {
69-
"axios": "^1.5.1",
69+
"axios": "^1.6.0",
7070
"del-cli": "^5.1.0",
71-
"esbuild": "^0.19.4",
72-
"nanostores": "^0.9.3",
71+
"esbuild": "^0.19.5",
72+
"nanostores": "^0.9.4",
7373
"openapi-typescript": "^6.7.0",
7474
"openapi-typescript-codegen": "^0.25.0",
7575
"openapi-typescript-fetch": "^1.1.3",

0 commit comments

Comments
 (0)