chore: enable @typescript-eslint/ban-ts-comment (#11326)

Author: sapphi-red

.eslintrc.cjs

@@ -61,7 +61,7 @@ module.exports = defineConfig({
     'node/no-unpublished-require': 'off',
     'node/no-unsupported-features/es-syntax': 'off',
 
-    '@typescript-eslint/ban-ts-comment': 'off', // TODO: we should turn this on in a new PR
+    '@typescript-eslint/ban-ts-comment': 'error',
     '@typescript-eslint/ban-types': 'off', // TODO: we should turn this on in a new PR
     '@typescript-eslint/explicit-module-boundary-types': [
       'error',
@@ -185,6 +185,16 @@ module.exports = defineConfig({
         '@typescript-eslint/no-empty-function': 'off',
       },
     },
+    {
+      files: [
+        'playground/tsconfig-json/**',
+        'playground/tsconfig-json-load-error/**',
+      ],
+      excludedFiles: '**/__tests__/**',
+      rules: {
+        '@typescript-eslint/ban-ts-comment': 'off',
+      },
+    },
     {
       files: ['*.js', '*.mjs', '*.cjs'],
       rules: {

packages/plugin-legacy/src/index.ts

@@ -18,7 +18,10 @@ import type {
   PreRenderedChunk,
   RenderedChunk,
 } from 'rollup'
-import type { PluginItem as BabelPlugin } from '@babel/core'
+import type {
+  PluginItem as BabelPlugin,
+  types as BabelTypes,
+} from '@babel/core'
 import colors from 'picocolors'
 import type { Options } from './types'
 
@@ -387,23 +390,22 @@ function viteLegacyPlugin(options: Options = {}): Plugin[] {
         return null
       }
 
-      // @ts-ignore avoid esbuild transform on legacy chunks since it produces
+      // @ts-expect-error avoid esbuild transform on legacy chunks since it produces
       // legacy-unsafe code - e.g. rewriting object properties into shorthands
       opts.__vite_skip_esbuild__ = true
 
-      // @ts-ignore force terser for legacy chunks. This only takes effect if
+      // @ts-expect-error force terser for legacy chunks. This only takes effect if
       // minification isn't disabled, because that leaves out the terser plugin
       // entirely.
       opts.__vite_force_terser__ = true
 
-      // @ts-ignore
-      // In the `generateBundle` hook,
+      // @ts-expect-error In the `generateBundle` hook,
       // we'll delete the assets from the legacy bundle to avoid emitting duplicate assets.
       // But that's still a waste of computing resource.
       // So we add this flag to avoid emitting the asset in the first place whenever possible.
       opts.__vite_skip_asset_emit__ = true
 
-      // @ts-ignore avoid emitting assets for legacy bundle
+      // avoid emitting assets for legacy bundle
       const needPolyfills =
         options.polyfills !== false && !Array.isArray(options.polyfills)
 
@@ -752,12 +754,11 @@ function isLegacyBundle(
 function recordAndRemovePolyfillBabelPlugin(
   polyfills: Set<string>,
 ): BabelPlugin {
-  return ({ types: t }): BabelPlugin => ({
+  return ({ types: t }: { types: typeof BabelTypes }): BabelPlugin => ({
     name: 'vite-remove-polyfill-import',
     post({ path }) {
       path.get('body').forEach((p) => {
-        if (t.isImportDeclaration(p)) {
-          // @ts-expect-error
+        if (t.isImportDeclaration(p.node)) {
           polyfills.add(p.node.source.value)
           p.remove()
         }

packages/vite/src/node/__tests__/plugins/css.spec.ts

@@ -209,7 +209,7 @@ async function createCssPluginTransform(
   const config = await resolveConfig(inlineConfig, 'serve')
   const { transform, buildStart } = cssPlugin(config)
 
-  // @ts-expect-error
+  // @ts-expect-error buildStart is function
   await buildStart.call({})
 
   const mockFs = vi
@@ -221,7 +221,7 @@ async function createCssPluginTransform(
 
   return {
     async transform(code: string, id: string) {
-      // @ts-expect-error
+      // @ts-expect-error transform is function
       return await transform.call(
         {
           addWatchFile() {

packages/vite/src/node/build.ts

@@ -546,8 +546,7 @@ async function doBuild(
 
   try {
     const buildOutputOptions = (output: OutputOptions = {}): OutputOptions => {
-      // See https://github.com/vitejs/vite/issues/5812#issuecomment-984345618
-      // @ts-ignore
+      // @ts-expect-error See https://github.com/vitejs/vite/issues/5812#issuecomment-984345618
       if (output.output) {
         config.logger.warn(
           `You've set "rollupOptions.output.output" in your config. ` +

packages/vite/src/node/cli.ts

@@ -1,7 +1,6 @@
 import path from 'node:path'
 import fs from 'node:fs'
 import { performance } from 'node:perf_hooks'
-import type { Session } from 'node:inspector'
 import { cac } from 'cac'
 import colors from 'picocolors'
 import type { BuildOptions } from './build'
@@ -32,8 +31,7 @@ interface GlobalCLIOptions {
   force?: boolean
 }
 
-// @ts-ignore
-let profileSession: Session | undefined = global.__vite_profile_session
+let profileSession = global.__vite_profile_session
 let profileCount = 0
 
 export const stopProfiler = (
@@ -141,7 +139,6 @@ cli
 
       const info = server.config.logger.info
 
-      // @ts-ignore
       const viteStartTime = global.__vite_start_time ?? false
       const startupDurationString = viteStartTime
         ? colors.dim(

packages/vite/src/node/config.ts

@@ -462,18 +462,13 @@ export async function resolveConfig(
   )
 
   const clientAlias = [
-    { find: /^\/?@vite\/env/, replacement: () => ENV_ENTRY },
-    { find: /^\/?@vite\/client/, replacement: () => CLIENT_ENTRY },
+    { find: /^\/?@vite\/env/, replacement: ENV_ENTRY },
+    { find: /^\/?@vite\/client/, replacement: CLIENT_ENTRY },
   ]
 
   // resolve alias with internal client alias
   const resolvedAlias = normalizeAlias(
-    mergeAlias(
-      // @ts-ignore because @rollup/plugin-alias' type doesn't allow function
-      // replacement, but its implementation does work with function values.
-      clientAlias,
-      config.resolve?.alias || [],
-    ),
+    mergeAlias(clientAlias, config.resolve?.alias || []),
   )
 
   const resolveOptions: ResolvedConfig['resolve'] = {

packages/vite/src/node/optimizer/optimizer.ts

@@ -466,7 +466,7 @@ async function createDepsOptimizer(
     }
 
     currentlyProcessing = false
-    // @ts-ignore
+    // @ts-expect-error `enqueuedRerun` could exist because `debouncedProcessing` may run while awaited
     enqueuedRerun?.()
   }
 

packages/vite/src/node/plugins/css.ts

@@ -623,7 +623,7 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
     },
 
     async generateBundle(opts, bundle) {
-      // @ts-ignore asset emits are skipped in legacy bundle
+      // @ts-expect-error asset emits are skipped in legacy bundle
       if (opts.__vite_skip_asset_emit__) {
         return
       }
@@ -1149,7 +1149,6 @@ async function resolvePostcssConfig(
     const searchPath =
       typeof inlineOptions === 'string' ? inlineOptions : config.root
     try {
-      // @ts-ignore
       result = await postcssrc({}, searchPath)
     } catch (e) {
       if (!/No PostCSS Config found/.test(e.message)) {
@@ -1508,6 +1507,9 @@ function loadPreprocessor(
   }
 }
 
+declare const window: unknown | undefined
+declare const location: { href: string } | undefined
+
 // in unix, scss might append `location.href` in environments that shim `location`
 // see https://github.com/sass/dart-sass/issues/710
 function cleanScssBugUrl(url: string) {
@@ -1533,12 +1535,10 @@ function fixScssBugImportValue(
     typeof window !== 'undefined' &&
     typeof location !== 'undefined' &&
     data &&
-    // @ts-expect-error
-    data.file &&
-    // @ts-expect-error
-    data.contents == null
+    'file' in data &&
+    (!('contents' in data) || data.contents == null)
   ) {
-    // @ts-expect-error
+    // @ts-expect-error we need to preserve file property for HMR
     data.contents = fs.readFileSync(data.file, 'utf-8')
   }
   return data

packages/vite/src/node/plugins/esbuild.ts

@@ -112,7 +112,7 @@ export async function transformWithEsbuild(
 
       for (const field of meaningfulFields) {
         if (field in loadedCompilerOptions) {
-          // @ts-ignore TypeScript can't tell they are of the same type
+          // @ts-expect-error TypeScript can't tell they are of the same type
           compilerOptionsForFile[field] = loadedCompilerOptions[field]
         }
       }
@@ -297,7 +297,7 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => {
       await initTSConfck(config)
     },
     async renderChunk(code, chunk, opts) {
-      // @ts-ignore injected by @vitejs/plugin-legacy
+      // @ts-expect-error injected by @vitejs/plugin-legacy
       if (opts.__vite_skip_esbuild__) {
         return null
       }

packages/vite/src/node/plugins/importAnalysisBuild.ts

@@ -52,21 +52,20 @@ function toRelativePath(filename: string, importer: string) {
  */
 
 function detectScriptRel() {
-  // @ts-ignore
   const relList = document.createElement('link').relList
-  // @ts-ignore
   return relList && relList.supports && relList.supports('modulepreload')
     ? 'modulepreload'
     : 'preload'
 }
 
 declare const scriptRel: string
+declare const seen: Record<string, boolean>
 function preload(
   baseModule: () => Promise<{}>,
   deps?: string[],
   importerUrl?: string,
 ) {
-  // @ts-ignore
+  // @ts-expect-error __VITE_IS_MODERN__ will be replaced with boolean later
   if (!__VITE_IS_MODERN__ || !deps || deps.length === 0) {
     return baseModule()
   }
@@ -75,11 +74,9 @@ function preload(
 
   return Promise.all(
     deps.map((dep) => {
-      // @ts-ignore
+      // @ts-expect-error assetsURL is declared before preload.toString()
       dep = assetsURL(dep, importerUrl)
-      // @ts-ignore
       if (dep in seen) return
-      // @ts-ignore
       seen[dep] = true
       const isCss = dep.endsWith('.css')
       const cssSelector = isCss ? '[rel="stylesheet"]' : ''
@@ -101,16 +98,13 @@ function preload(
         return
       }
 
-      // @ts-ignore
       const link = document.createElement('link')
-      // @ts-ignore
       link.rel = isCss ? 'stylesheet' : scriptRel
       if (!isCss) {
         link.as = 'script'
         link.crossOrigin = ''
       }
       link.href = dep
-      // @ts-ignore
       document.head.appendChild(link)
       if (isCss) {
         return new Promise((res, rej) => {