ESLint: Ban @ts-ignore in favour of @ts-expect-error

[manzoorwanijk]

What?

Enables @typescript-eslint/ban-ts-comment so @ts-ignore is an error and @ts-expect-error requires a description, then fixes every existing violation.

Why?

@ts-ignore keeps silently passing once the error it was added for is gone, so stale suppressions accumulate unnoticed. @ts-expect-error fails when it is no longer needed, which surfaces them.

That is exactly what happened here: of the 117 @ts-ignore comments on trunk, 35 (30%) were stale — they suppressed nothing. Several also carried reasons that were no longer true, such as // @ts-ignore - Preferences package is not typed when @wordpress/preferences has been typed for a while.

Follow up to #80832 (comment).

How?

The rule is added to the repo config in tools/eslint/config.mjs. Defaults are kept, so @ts-expect-error needs a description of at least 3 characters. It applies to .js as well as .ts/.tsx — the rule only walks the comment list, so it works under the Babel parser too.

The 177 violations were resolved as:

	Count
@ts-ignore converted to @ts-expect-error with a description	82
@ts-ignore deleted as stale	35
Bare @ts-expect-error given a description	60

Each description states the reason TypeScript actually reports, taken from the compiler rather than guessed. Every directive was temporarily neutralised and tsc --build re-run to capture the real diagnostic, then restored.

Some paths get no signal from the build: tsconfig.base.json excludes **/test/**, and tools/ and routes/ are not in any TypeScript project. Directives there were checked against a temporary project extending tsconfig.base.json, and against lib.dom.d.ts and package metadata where that was the relevant fact.

Unrelated drive-by: one pre-existing import/order error in packages/components/src/button/test/index.tsx is fixed, because the pre-commit hook lints staged files under the strict config and it blocked the commit.

Testing Instructions

1. npm run lint:js passes.
2. npx tsc --build from the repo root exits cleanly.
3. Add // @ts-ignore anywhere and confirm linting fails with a message pointing at @ts-expect-error.
4. Add a bare // @ts-expect-error and confirm linting asks for a description.

Use of AI Tools

Authored with Claude Code, then audited with Codex specifically for the factual accuracy of the new comments. That audit caught descriptions that named the wrong cause and four directives that suppressed nothing; each finding was re-verified against real tsc output before being applied. I have reviewed and take responsibility for the changes.

[github-actions]

Size Change: 0 B

Total Size: 7.81 MB

_{compressed-size-action}

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org>
Co-authored-by: aduth <aduth@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Flaky tests detected in 99ac5c3.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/30903043886
📝 Reported issues:

• [Flaky Test] does not dirty related template parts or navigation menus when rendered in the post editor #79294 in /test/e2e/specs/editor/blocks/navigation-passive-rendering.spec.js

[aduth]

This is great to keep our disabling truthful to the code 👍

I left some comments about specific instances as I was reviewing them. I don't expect those to be addressed here; in fact, I'd prefer they weren't addressed here, and I understand the backfilled descriptions to justify their existence as intended at the time. I'm hopeful that requiring descriptions will push against the instinct to add disabling comments when a better solution might exist.

[aduth]

For follow-up: The newest version does!

[manzoorwanijk]

Addressed in #81199

[aduth]

This code is a little odd, both in terms of whether it even makes sense to pass through all these props blindly, or if we could be doing a better job of typing it such that we don't need to disable. Cursory review with AI shows it might be possible but not entirely straight-forward.

[aduth]

This looks like case where we're "doing it wrong" and could do this in a type-compliant way.

Suggested change

	// @ts-expect-error `cursor` is typed as `string`, but `null` clears it.
	document.documentElement.style.cursor = null;
	document.documentElement.style.cursor = '';

Setting to empty string removes the proeprty.

Source:

If value is the empty string, invoke removeProperty() with property as argument and return.

https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty

Setting to null is the same as setting to empty string.

Source:

Instead of being stringified to "null", which is the default, [a JavaScript null] will be converted to the empty string.

https://webidl.spec.whatwg.org/#LegacyNullToEmptyString

Suggested change

	// @ts-expect-error `cursor` is typed as `string`, but `null` clears it.
	document.documentElement.style.cursor = null;
	document.documentElement.style.removeProperty( 'cursor' );

Ref: https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleDeclaration/removeProperty

[aduth]

We should think about how to support this in a cross-React 18/19 compatible way (cc @jsnajdr ).

[aduth]

This seems like it might just be incorrect typings on label ?

It's typed as a string:

gutenberg/packages/dataviews/src/types/field-api.ts

Line 21 in ad0af52

label: string;

But then it's referenced as an array of two members, as we're constructing here:

gutenberg/packages/dataviews/src/utils/operators.tsx

Lines 230 to 231 in ad0af52

	activeElements[ 0 ].label[ 0 ],
	activeElements[ 0 ].label[ 1 ]

I guess it's a little tricky, because other places treat it as a string, it's just this "between" operator that's doing something unique. string | string[] could work as a type, but it overpromises for those other cases.

[aduth]

This could probably be addressed by changing the typing above for isVisibleFlag: string to isVisibleFlag: 'showTitle' | 'showMedia' | 'showDescription' or some variation of keyof typeof view that reflects the fields we're expecting.

[aduth]

Maybe we should update the types to include __experimentalDefault ? We could debate whether to add types for experimental attributes, but there's plenty of precedent already in the same file.

[aduth]

We could probably address this with effective narrowing / type predicates.

[manzoorwanijk]

I left some comments about specific instances as I was reviewing them. I don't expect those to be addressed here; in fact, I'd prefer they weren't addressed here, and I understand the backfilled descriptions to justify their existence as intended at the time.

Yes, my intension was to keep this PR free from any runtime changes and focus purely on getting the rule into trunk.

I'm hopeful that requiring descriptions will push against the instinct to add disabling comments when a better solution might exist.

My opinion is that the forced description will make people/agents think about the justification and may be that can help the root cause.

[manzoorwanijk]

@aduth I am merging this as is and then I will create a follow up PR or two to adress the concerns.

[manzoorwanijk]

I have created the following PRs as follow ups to fix the easy ones here

• Blocks: Update hpq to 1.4.0 and drop the ts-expect-error suppressions #81199
• Types: Replace remaining @ts-expect-error suppressions with proper typing #81200
• Blocks: Declare the @types/react-is devDependency #81203