👉 This has since been posted to DefinitelyTyped's GitHub Discussions: DefinitelyTyped/DefinitelyTyped#65993 👈
👋 Hi all! I've been working with the TypeScript team on overhauling DefinitelyTyped's usage of formatting and linting tools. Alongside the effort to finally migrate from TSLint to ESLint (https://typescript-eslint.io/linting/troubleshooting/tslint -> microsoft/DefinitelyTyped-tools#648), we'd like to align with how much of the industry & many linter maintainers recommend using a formatter vs. a linter (myself included).
This is a surfacing of a plan we've formulated that we think will get DefinitelyTyped onto using a proper formatter for formatting. I'd very much like to see the community's reaction: does this seem reasonable to you? What edge cases are we missing? Will this offend your sensibilities as a programmer?
DefinitelyTyped today only utilizes a few ESLint rules for code formatting. It does not use Prettier, dprint, or other dedicated formatting tools.
We'd like to move DefinitelyTyped onto one of those tools for the following benefits:
- Consistency: so contributors don't have to adjust to different formatting preferences per types package.
- Dev time: formatters save contributor time from being spent on manually touching up files.
- Diff sizes: so developers stop accidentally (or intentionally) reformatting entire files in the course of making an unrelated change.
- Easier TSLint to ESLint migration: removing usage of formatting rules reduces the amount of logic we'll need to port from TSLint to ESLint
✋ Please do not reply to this with formatting opinions. We know using a formatter instead of a linter for formatting can be a contentious choice. Bikeshedding on nuances of whitespace preferences is not useful.
- Determine the formatter choice and formatting configuration
- Create and merge a DefinitelyTyped-tools pull request that removes all formatting rules
- Simultaneously to (2), create and merge a DefinitelyTyped pull request that:
- Runs the formatter on all files in DefinitelyTyped
- Adds checking in formatting fixes to the merge bot
- Adds a GitHub Action that makes a new commit to format all files if needed after merges to
master
- Create and merge a DefinitelyTyped pull request that adds a
.git-blame-ignore-revsfile to ignore the large formatting commit
The two most common standalone formatters in the JavaScript/TypeScript ecosystem today are:
We plan on using dprint for formatting for its superior performance.
A quick comparison of running both tools and pprettier on DefinitelyTyped shows a 10 second vs. minutes-scale difference:
npx dprint fmt 141.52s user 9.29s system 1481% cpu 10.177 total
npx pprettier --write './types/**/*.{ts,mts,cts}' 8.97s user 3.18s system 21% cpu 56.725 total
npx prettier -w ./types 361.28s user 35.60s system 126% cpu 5:13.36 total
dprint is also able to use a cacheable file cache, which drops times down to under a second. We may look into using it in CI as a followup performance optimization.
Additionally, running dprint on as large a repository as DefinitelyTyped will provide it and SWC with helpful community dogfooding (e.g. swc-project/swc#7187, dprint/dprint-plugin-typescript#533). Note that dprint is essentially deno fmt, so we're not worried about its ability to handle all of DefinitelyTyped.
DefinitelyTyped today already has a .prettierrc.json Prettier configuration file:
{
"$schema": "http://json.schemastore.org/prettierrc",
"arrowParens": "avoid",
"singleQuote": true,
"trailingComma": "all",
"tabWidth": 4,
"printWidth": 120
}Our plan is to use the closest equivalent dprint TypeScript configuration. This proposal does not include work to change those settings. No set of settings is perfect or will satisfy all users.
Pull request CI builds will not be blocked on formatting, so as to not inconvenience contributors who don't run a formatter. However, two parts of the plan ensure the master branch stays fully formatted:
- Most PRs are merged by the DT bot: which can apply apply formatting on each PR as part of its existing merge process.
- For manual merges done by maintainers: the action that runs on merges to
masterwill capture any formatting mishaps
As a follow-up, we may want to create a general-purpose PR bot to make it easier for contributors to apply formatting to a PR on their behalf. This is out of scope for the current proposal.
Yes, but considering the advantages mentioned earlier in this proposal, we strongly advise against doing so. Individual files that are auto-generated can use configuration file excludes and/or // dprint-ignore-file file ignore comments to disable the formatter.
Most users would not notice a difference in performance between dprint and Prettier, as most files aren't large enough (thousands of lines long) to cause a significant difference. However, the CI runs on the master branch should ideally be as fast as possible to minimize the chance of users branching off a master commit with incorrectly unformatted code.
One potential side effect of switching to a dedicated formatter is that individual projects that prefer using tabs for accessibility may find it harder to do so in the project. If that describes you, please post here and let us know your use case and why you need to use tabs.
Projects that want to still use tabs can always disable the formatter. Again, please let us know - if this would inconvenience you we can try to brainstorm a workaround.
Today, only 19 types directories use tabs - as determined by finding all types/ directories that have any file with a number of \n\ts greater than 1% of its total \n count. Therefore, this proposal is not changing how the vast majority (>99.9%) of types packages use spaces.
Script run to find types packages that use tabs
import fs from 'node:fs/promises';
import path from 'node:path';
async function checkDirectoryPathForTabs(directoryPath) {
const directoryItems = await readdirSafe(directoryPath);
for (const directoryItem of directoryItems ?? []) {
const directoryItemPath = path.join(directoryPath, directoryItem);
if (!directoryItemPath.includes('.ts')) {
continue;
}
const contents = await readFileSafe(directoryItemPath);
if (!contents) {
continue;
}
const newlineTabsCount = contents.match(/\n\t/g)?.length ?? 0;
if (!newlineTabsCount) {
continue;
}
const newlineCount = contents.match(/\n/g)?.length ?? 0;
if (newlineTabsCount > newlineCount / 100) {
console.log(directoryPath)
return;
}
}
}
async function readdirSafe(dirPath) {
try {
return await fs.readdir(dirPath);
} catch {
return undefined;
}
}
async function readFileSafe(filePath) {
try {
return (await fs.readFile(filePath)).toString();
} catch {
return undefined;
}
}
for (const directory of await fs.readdir('./types')) {
const directoryPath = path.join('./types', directory);
await checkDirectoryPathForTabs(directoryPath)
}Package names logged by that script
types/anime-quotes-api
types/arcgis-js-api
types/bootbox
types/ceddl__ceddl-polyfill
types/classificator
types/dns-packet
types/do-not-zip
types/ej.web.all
types/gulp-help
types/gulp-inject
types/js2coffee
types/json-query
types/qbittorrent-api-v2
types/react-leaflet-markercluster
types/reefjs
types/steelseries
types/titanium
types/uuid4
types/vscode
A formatter quickly (re-)formats code without adjusting any code logic. A linter runs a set of discrete rules that each flag -and optionally suggest fixes- for a specific kind of code defect.
See https://github.com/readme/guides/formatters-linters-compilers for more context.
This proposal came out of a group thread with discussion from:
- Piotr Błażejewicz
- David Sherret
- Sebastian Silbermann
- Jake Bailey
- John Reilly
- Josh Goldberg
- Nathan Shively-Sanders
Appreciation in particular to Jake Bailey and Nathan Shively-Sanders for advising from the TypeScript team, and David Sherret from dprint.
Adjusted for those, thanks!
I'm always hesitant to phrase myself as a reputable source. Partially out of shyness, and partially because I know there are quite a few linter-area folks more tenured than I who have different or less emphatic opinions.