-
-
Notifications
You must be signed in to change notification settings - Fork 9
Check type declarations with JSDoc comments #10
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
Conversation
|
Thanks for working on this. Can it be done by adding a |
047b7ac to
3131a1f
Compare
Yeah course, I've pushed up a new .gitignore to show them:
I've used
|
index.d.cts
Outdated
| declare const PRETTIER_ASYNC_FUNCTIONS: readonly [ | ||
| "formatWithCursor", | ||
| "format", | ||
| "check", | ||
| "resolveConfig", | ||
| "resolveConfigFile", | ||
| "clearConfigCache", | ||
| "getFileInfo", | ||
| "getSupportInfo", | ||
| ]; | ||
| declare const PRETTIER_STATIC_PROPERTIES: readonly ["version", "util", "doc"]; |
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.
I'm not familiar with types, but why are these needed?
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.
They're not publicly exported, but I've added /** @type {const} */ to these two:
PRETTIER_ASYNC_FUNCTIONS
PRETTIER_STATIC_PROPERTIESWhen a function is called or property is accessed that doesn't exist (or renamed) it'll validate against:
FunctionName in typeof PRETTIER_ASYNC_FUNCTIONS[number]
PropertyName in typeof PRETTIER_STATIC_PROPERTIES[number]Otherwise we'd have to duplicate them in quite a few places:
FunctionName extends "formatWithCursor" | "format" | "check" | "resolveConfig" | "resolveConfigFile" | "clearConfigCache" | "getFileInfo" | "getSupportInfo"
Property extends "version" | "util" | "doc"These consts are used to populate the functions/properties available in editors:
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.
Appreciate they're not needed externally so tried adding @internal
But when setting tsconfig.json "stripInternal": true it didn't work due to:
- Support stripInternal for types in JSDoc microsoft/TypeScript#46407
stripInternalshould work on single-line comments microsoft/TypeScript#46515
One of the downsides to using the compiler with JSDoc 😔
index.d.cts
Outdated
| @@ -0,0 +1,54 @@ | |||
| declare const _exports: PrettierSync; | |||
| export = _exports; | |||
| export type Prettier = typeof import("prettier"); | |||
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.
Is it possible to do something like
import type Prettier from "prettier";?
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.
Let me look into it
It's the tsc compiler automatically exporting @typedef by default
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.
How about hand-writing code like this
import type {
version,
util,
doc,
formatWithCursor,
format,
check,
resolveConfig,
resolveConfigFile,
getFileInfo,
getSupportInfo,
} from "prettier";
type SynchronizedPrettier = {
// Prettier static properties
version: typeof version;
util: typeof util;
doc: typeof doc;
// Prettier functions
formatWithCursor: Awaited<ReturnType<typeof formatWithCursor>>;
format: Awaited<ReturnType<typeof format>>;
check: Awaited<ReturnType<typeof check>>;
resolveConfig: Awaited<ReturnType<typeof resolveConfig>>;
resolveConfigFile: Awaited<ReturnType<typeof resolveConfigFile>>;
getFileInfo: Awaited<ReturnType<typeof getFileInfo>>;
getSupportInfo: Awaited<ReturnType<typeof getSupportInfo>>;
};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.
Entirely up to you 😊
With JSDoc and checkJs enabled (without switching to TypeScript) you'll be able to use code completion, see documentation in tooltips, and spot and errors and warnings as-you-type—in this repo too
Plus it lets the compiler validate that type declarations are correct when prettier "./src/index.d.ts" types change without having to keep types synchronised between repos by hand
tsconfig.json
{
"compilerOptions": {
"allowJs": true,
"checkJs": true
}
}I'd be more than happy if you used this PR for guidance to add them manually 👍
Let me know if you'd like anything splitting out
Update: Split out manually in 7d922c2
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './index.cjs' is not defined by "exports" in /path/to/example/node_modules/@prettier/sync/package.json imported from /path/to/example/index.mjs
…ing type declarations.”
d310ad2 to
afd3bac
Compare
|
I have no experience with type declarations. @SimenB Can you help review? |
afd3bac to
7d922c2
Compare
|
@fisker I've removed all the unnecessary autogenerated lines from We're left with the good stuff like But I've kept the tsconfig.json and JSDoc comments so you get type checking locally too |
|
Brilliant, thanks @fisker Just a reminder that manually adding the types without a type declaration map means editor and IDE "Go to definition" will jump to the types rather than the original Prettier source code, should you want that in future Glad I could help though, appreciate it |

Great work on Prettier v3 🙌
Hope this PR is welcome, but I've added JSDoc + tsconfig.json to export type declarations
I followed the link in the Prettier 3.0: Hello, ECMAScript Modules! blog post after realising that the Prettier sync APIs would still be needed to avoid Nunjucks template changes for asynchronous rendering 😣
I'd be happy to make any tweaks, and wasn't sure if committing the types would be preferred
Closes: #8