-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Types: Enable incremental JSDoc type checking #13104
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ To some extent, this guide can be summarized as _make new code similar to existi | |
| - [Formatting](#formatting) | ||
| - [Spelling](#spelling) | ||
| - [Linting](#linting) | ||
| - [Type Checking](#type-checking) | ||
| - [Units](#units) | ||
| - [Basic Code Construction](#basic-code-construction) | ||
| - [Functions](#functions) | ||
|
|
@@ -172,6 +173,34 @@ try { | |
| /*eslint-enable no-empty*/ | ||
| ``` | ||
|
|
||
| ## Type Checking | ||
|
|
||
| We are incrementally adopting [type checking for JavaScript files](https://www.typescriptlang.org/docs/handbook/type-checking-javascript-files.html). As of January 2026, type checks are enabled on a file-by-file basis, with `// @ts-check` annotations at the top of a file used to opt in. As adoption progresses, we expect this pattern will eventually be flipped to an opt-opt annotation instead. To see type system hints and errors in some editors or IDEs, you may need to configure or install TypeScript language server support. When in doubt, VSCode supports TypeScript language services [by default](https://code.visualstudio.com/docs/nodejs/working-with-javascript). | ||
|
|
||
| **References:** | ||
|
|
||
| For developers already familiar with TypeScript, writing JavaScript with JSDoc type annotations has some differences. For a summary of the annotation syntax, and supported features in JSDoc vs. TSDoc, see: | ||
|
|
||
| - [Type Checking JavaScript Files \| TypeScript](https://www.typescriptlang.org/docs/handbook/type-checking-javascript-files.html) | ||
| - [JSDoc Reference \| TypeScript](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html) | ||
| - [JSDoc Cheatsheet and Type Safety Tricks](https://docs.joshuatz.com/cheatsheets/js/jsdoc/) by Joshua Tzucker | ||
|
Comment on lines
+184
to
+186
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to be careful about links like these. TS has sort of expanded or co-opted JSDoc for extra functionality that is not part of JSDoc itself. Our doc generation still uses pure JSDoc so we are beholden to only features it supports. The For example in the TS JSDoc Reference it mentions the Also the very first recommendation in the Cheatsheet shows how to typeguard functions using I bet we could make more edits to our JSDoc config/generation to adapt for more tags but I'm not sure the level of effort there. I mostly bring it up as a word of caution. Maybe all it needs is a warning that our doc gen still needs to work and to be careful. Or a comment that we need to restrict to only official JSDoc tags? Happy to discuss |
||
|
|
||
| **Guidelines:** | ||
|
|
||
| 1. **Incremental adoption with `@ts-check` annotations.** Enabling type checks for existing and new JavaScript files, using `// @ts-check` annotations, is encouraged but not required in the course of ongoing feature development and maintenance. If type-related changes are noisy enough to obscure the functional changes your PR, consider splitting the type fixes into a separate PR. | ||
| 2. **Common JavaScript/JSDoc obstacles.** JSDoc-based type checks have limited support for some coding patterns used in the CesiumJS codebase, such as classic prototype-based inheritance and `Object.defineProperties()`. In most cases these problems can be solved by refactoring with newer coding patterns, by hand or with tooling like [lebab](https://github.com/lebab/lebab). In other cases the solution may be less obvious, and a temporary skip-check annotation may be used to satisfy the type system. Example with lebab: | ||
| ```bash | ||
| npx lebab packages/engine/Source/Core/Cartesian2.js \ | ||
| -o packages/engine/Source/Core/Cartesian2.js --transform class | ||
| ``` | ||
| 3. **Prefer `@ts-expect-error` over `@ts-ignore`.** Sometimes a source file's types are _mostly_ consistent, but a few stubborn errors remain unsolved. These may be blocked by out-of-scope work, or may simply be unjustifiably difficult to solve in JSDoc-based types. Use `@ts-expect-error` annotations to relax the type system for specific lines. Unlike `@ts-ignore`, `@ts-expect-error` will raise errors when the line no longer contains an error, making it easier to clean up annotations later on. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this suggestion for forward compatibility and cleanup. (a quick search led to articles that agree too) Can we expand this to always include a reason with the disable comment? It also could be good to look at enforcing this programmatically in eslint. We're already using |
||
| 4. **Prefer `unknown` or `@ts-expect-error` over `any`.** Casting types to `any` forces them to be accepted everywhere, and effectively disables all type-checking dependent on that type. This tends to propagate throughout a type system, reducing the effectiveness of type checks, and should be avoided when possible. Prefer casting to `unknown`, or using a `@ts-expect-error` annotation. For example, when defining an object with string keys, with values whose types we don't care about, prefer `Record<string, unknown>` to `Record<string, any>`. | ||
| 5. **Type-only imports.** When JSDoc annotations depend on types not otherwise imported in a source file, it will be necessary to tell TypeScript where to find them. To avoid otherwise-unused imports, use type-only imports in separate one-line JSDoc comments: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The very first google search for "import type in JSDoc" points to an older syntax that still works for the TS side of things /**
* @typedef {import('./File1.js').MyObject1} MyObject1
*/However this is not valid JSDoc and will break things. I think it's worth calling out here to not use this syntax and instead always rely only on the |
||
| ```javascript | ||
| /** @import Cartesian3 from './Cartesian3.js'; */ | ||
| /** @import Cartesian4 from './Cartesian4.js'; */ | ||
| ``` | ||
|
|
||
| ## Units | ||
|
|
||
| - Cesium uses SI units: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| export default { | ||
| // https://github.com/lint-staged/lint-staged#how-can-i-resolve-typescript-tsc-ignoring-tsconfigjson-when-lint-staged-runs-via-husky-hooks | ||
| "*.{js,cjs,mjs,ts,cts,mts}": [() => "tsc"], | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| { | ||
| "include": ["Source/**/*.js"], | ||
| "compilerOptions": { | ||
| // Module configuration. | ||
| "moduleResolution": "bundler", | ||
| "module": "ES2022", | ||
| "target": "ES2022", | ||
|
|
||
| // I/O. | ||
| "noEmit": true, | ||
| "allowJs": true, | ||
|
|
||
| // Disabled by default. Individual JS files may opt-in to type checking | ||
| // by including a `// @ts-check` comment at top of file. | ||
| "checkJs": false, | ||
|
|
||
| // Checking declarations in dependencies is less important and less | ||
| // actionable than checking source JS. Skip until checkJS is on, at least. | ||
| "skipLibCheck": true | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo