Update (October 9th, 2023): This work is done! As of #66235's merge on October 6th, all DefinitelyTyped's types files are formatted with dprint (with a small number of temporary exceptions for build issues).

Context

👋 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's Formatting Context

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.

The Plan

1. Determine the formatter choice and formatting configuration
2. Create and merge a DefinitelyTyped-tools pull request that removes all formatting rules
3. Simultaneously to (2), create and merge DefinitelyTyped pull requests 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
4. Create and merge a DefinitelyTyped pull request that adds a .git-blame-ignore-revs file to ignore the large formatting commits

Edit August 10th: @jakebailey, @sandersn, and I discussed the "check in formatting fixes to the merge bot" bit in more detail. It's a complicated work item given GitHub's security concerns & approval flows, and dprint performance has been consistently excellent. We're going to table it until it's shown to be needed (i.e. conflicts from master temporarily being unformatted start arising frequently).

Formatter Choice: dprint

The two most common standalone formatters in the JavaScript/TypeScript ecosystem today are:

• dprint: much faster, but less popular
• Prettier: more popular, but slower

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.

Formatting Configuration

DefinitelyTyped today already has a .prettierrc.json Prettier configuration file:

{
  "$schema": "http://json.schemastore.org/prettierrc",
  // Edit August 29th: Removed overrides. See below.
  // "arrowParens": "avoid",
  // "singleQuote": true,
  // "trailingComma": "onlyMultiLine",
  "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.

Edit August 29th: Following #66238 (comment) & #66238 (comment), I removed the style overrides.

FAQs

Explainer: When Formatting Runs

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 the master branch will be fully formatted most of the time:

• 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 master will capture any formatting mishaps The action that runs after merges to master will quickly format it if needed.

Edit August 10th: see earlier note on tabling mergebot changes.

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.

Can Files Opt Out of Formatting?

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.

Edit July 31st, 2023: As of dprint/dprint#719, dprint also supports nested configurations. Individual types/ directories can reconfigure dprint if they absolutely need. We'll set this up for directories that are fully auto-generated.

Is Formatting Performance Important?

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.

Tabs or Spaces?

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. We'll seed the configuration file to disable formatting for existing packages that use tabs. Please let us know if that's not enough: we can try to brainstorm a workaround, such as adding the ability to have project-specific overrides to the formatter.

Edit July 31st, 2023: As of dprint/dprint#719, dprint also supports nested configurations. Individual types/ directories can reconfigure dprint if they prefer need. Our initial PR that converts DefinitelyTyped to dprint will use tabs in all packages that seem to prefer them.

Today, only 19 packages 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. Doing so is a potential large follow-up task we can look into.

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)
}

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

What About .json and .md Files?

(added July 31st 2023)

This proposal doesn't intend to format those files with the formatter. We may look into that as a followup.

What's the Difference Between a Formatter and a Linter?

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.

Why split up the PRs so much?

Splitting up changes is easier on tooling:

• It's easier to review the diffs on github.com (the Files view slows down at hundreds of files)
• Blocking CI failures for a package will only block that package's PR
  • Example difficulty: header-parser should mention which file failed to parse microsoft/DefinitelyTyped-tools#719
• If we need to revert later on, we'll be able to do so more granularly

Our splitting strategy was:

1. Find the biggest packages (Using a formatter for formatting (instead of lint rules) #65993 (comment)) and send their PRs early
2. Then, send a PR for each letter of the alphabet
   • This is arbitrary but makes it easier to associate PRs with their related packages

Thanks

This proposal came out of a group thread with discussion from:

• David Sherret
• Jake Bailey
• John Reilly
• Josh Goldberg
• Nathan Shively-Sanders
• Piotr Błażejewicz
• Sebastian Silbermann

Appreciation in particular to Jake Bailey and Nathan Shively-Sanders for advising from the TypeScript team, and David Sherret from dprint.