Remove ts-ignore where possible (#10541)

* Remove ts-ignore where possible And replace by typecasts * More accurate types * bend cliententries in a correct shape earlier on * comment becomes unnecessary * add webpack overload to allow for the next.js use case * Avoid changing public interface Co-authored-by: Joe Haddad <[email protected]>
vercel · Feb 17, 2020 · 3f691ea · 3f691ea
1 parent f4c5a95
commit 3f691ea
Show file tree

Hide file tree

Showing 13 changed files with 66 additions and 35 deletions.
diff --git a/packages/next/build/babel/plugins/react-loadable-plugin.ts b/packages/next/build/babel/plugins/react-loadable-plugin.ts
@@ -85,8 +85,6 @@ export default function({ types: t }: { types: typeof BabelTypes }): PluginObj {
               callExpression.node.arguments.push(t.objectExpression([]))
             }
             // This is needed as the code is modified above
-            // TODO: remove ignore statement
-            // @ts-ignore
             args = callExpression.get('arguments')
             loader = args[0]
             options = args[1]

diff --git a/packages/next/build/compiler.ts b/packages/next/build/compiler.ts
@@ -26,7 +26,6 @@ export function runCompiler(
   config: webpack.Configuration | webpack.Configuration[]
 ): Promise<CompilerResult> {
   return new Promise(async (resolve, reject) => {
-    // @ts-ignore webpack allows both a single config or array of configs
     const compiler = webpack(config)
     compiler.run((err: Error, statsOrMultiStats: any) => {
       if (err) {

diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts
@@ -1,7 +1,6 @@
 import crypto from 'crypto'
 import ForkTsCheckerWebpackPlugin from 'fork-ts-checker-webpack-plugin'
 import path from 'path'
-// @ts-ignore: Currently missing types
 import PnpWebpackPlugin from 'pnp-webpack-plugin'
 import webpack from 'webpack'
 import {
@@ -29,7 +28,6 @@ import {
 } from './plugins/collect-plugins'
 import { build as buildConfiguration } from './webpack/config'
 import { __overrideCssConfiguration } from './webpack/config/blocks/css/overrideCssConfiguration'
-// @ts-ignore: JS file
 import { pluginLoaderOptions } from './webpack/loaders/next-plugin-loader'
 import BuildManifestPlugin from './webpack/plugins/build-manifest-plugin'
 import ChunkNamesPlugin from './webpack/plugins/chunk-names-plugin'
@@ -113,6 +111,12 @@ function getOptimizedAliases(
   )
 }
 
+type ClientEntries = {
+  'main.js': string[]
+} & {
+  [key: string]: string
+}
+
 export default async function getBaseWebpackConfig(
   dir: string,
   {
@@ -197,7 +201,7 @@ export default async function getBaseWebpackConfig(
   const outputPath = path.join(distDir, isServer ? outputDir : '')
   const totalPages = Object.keys(entrypoints).length
   const clientEntries = !isServer
-    ? {
+    ? ({
         // Backwards compatibility
         'main.js': [],
         [CLIENT_STATIC_FILES_RUNTIME_MAIN]:
@@ -215,7 +219,7 @@ export default async function getBaseWebpackConfig(
             ? 'polyfills-nomodule.js'
             : 'polyfills.js'
         ),
-      }
+      } as ClientEntries)
     : undefined
 
   let typeScriptPath
@@ -652,7 +656,6 @@ export default async function getBaseWebpackConfig(
       ],
       plugins: [PnpWebpackPlugin],
     },
-    // @ts-ignore this is filtered
     module: {
       rules: [
         {
@@ -902,8 +905,7 @@ export default async function getBaseWebpackConfig(
       webpack,
     })
 
-    // @ts-ignore: Property 'then' does not exist on type 'Configuration'
-    if (typeof webpackConfig.then === 'function') {
+    if (typeof (webpackConfig as any).then === 'function') {
       console.warn(
         '> Promise returned in next config. https://err.sh/zeit/next.js/promise-in-next-config'
       )
@@ -1109,7 +1111,6 @@ export default async function getBaseWebpackConfig(
       // Server compilation doesn't have main.js
       if (clientEntries && entry['main.js'] && entry['main.js'].length > 0) {
         const originalFile = clientEntries[CLIENT_STATIC_FILES_RUNTIME_MAIN]
-        // @ts-ignore TODO: investigate type error
         entry[CLIENT_STATIC_FILES_RUNTIME_MAIN] = [
           ...entry['main.js'],
           originalFile,
@@ -1122,8 +1123,8 @@ export default async function getBaseWebpackConfig(
   }
 
   if (!dev) {
-    // @ts-ignore entry is always a function
-    webpackConfig.entry = await webpackConfig.entry()
+    // entry is always a function
+    webpackConfig.entry = await (webpackConfig.entry as webpack.EntryFunc)()
   }
 
   return webpackConfig

diff --git a/packages/next/build/webpack/plugins/webpack-conformance-plugin/index.ts b/packages/next/build/webpack/plugins/webpack-conformance-plugin/index.ts
@@ -88,16 +88,17 @@ export default class WebpackConformancePlugin {
   private parserHandler = (factory: compilation.NormalModuleFactory): void => {
     const JS_TYPES = ['auto', 'esm', 'dynamic']
     const collectedVisitors: Map<string, [NodeInspector?]> = new Map()
-    // Collect all intereseted visitors from all tests.
+    // Collect all interested visitors from all tests.
     this.tests.forEach(test => {
       if (test.getAstNode) {
         const getAstNodeCallbacks: IGetAstNodeResult[] = test.getAstNode()
         getAstNodeCallbacks.forEach(result => {
           if (!collectedVisitors.has(result.visitor)) {
             collectedVisitors.set(result.visitor, [])
           }
-          // @ts-ignore
-          collectedVisitors.get(result.visitor).push(result.inspectNode)
+          ;(collectedVisitors.get(result.visitor) as NodeInspector[]).push(
+            result.inspectNode
+          )
         })
       }
     })

diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx
@@ -161,8 +161,7 @@ class Link extends Component<LinkProps> {
   })
 
   linkClicked = (e: React.MouseEvent) => {
-    // @ts-ignore target exists on currentTarget
-    const { nodeName, target } = e.currentTarget
+    const { nodeName, target } = e.currentTarget as HTMLAnchorElement
     if (
       nodeName === 'A' &&
       ((target && target !== '_self') ||

diff --git a/packages/next/lib/resolve-request.ts b/packages/next/lib/resolve-request.ts
@@ -5,7 +5,6 @@ export function resolveRequest(req: string, issuer: string) {
   // The `resolve` package is prebuilt through ncc, which prevents
   // PnP from being able to inject itself into it. To circumvent
   // this, we simply use PnP directly when available.
-  // @ts-ignore
   if (process.versions.pnp) {
     const { resolveRequest } = require(`pnpapi`)
     return resolveRequest(req, issuer, { considerBuiltins: false })

diff --git a/packages/next/lib/verifyTypeScriptSetup.ts b/packages/next/lib/verifyTypeScriptSetup.ts
@@ -123,7 +123,6 @@ export async function verifyTypeScriptSetup(
   }
 
   const tsPath = await checkDependencies({ dir, isYarn })
-  // @ts-ignore
   const ts = (await import(tsPath)) as typeof import('typescript')
 
   const compilerOptions: any = {

diff --git a/packages/next/next-server/lib/loadable-context.ts b/packages/next/next-server/lib/loadable-context.ts
@@ -2,7 +2,4 @@ import * as React from 'react'
 
 type CaptureFn = (moduleName: string) => void
 
-// @ts-ignore for some reason the React types don't like this, but it's correct.
-export const LoadableContext: React.Context<CaptureFn | null> = React.createContext(
-  null
-)
+export const LoadableContext = React.createContext<CaptureFn | null>(null)
diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts
@@ -17,8 +17,8 @@ import { getRouteMatcher } from './utils/route-matcher'
 import { getRouteRegex } from './utils/route-regex'
 
 function addBasePath(path: string): string {
-  // @ts-ignore variable is always a string
-  const p: string = process.env.__NEXT_ROUTER_BASEPATH
+  // variable is always a string
+  const p = process.env.__NEXT_ROUTER_BASEPATH as string
   return path.indexOf(p) !== 0 ? p + path : path
 }
 
@@ -68,6 +68,8 @@ type BeforePopStateCallback = (state: any) => boolean
 
 type ComponentLoadCancel = (() => void) | null
 
+type HistoryMethod = 'replaceState' | 'pushState'
+
 function fetchNextData(
   pathname: string,
   query: ParsedUrlQuery | null,
@@ -313,7 +315,12 @@ export default class Router implements BaseRouter {
     return this.change('replaceState', url, as, options)
   }
 
-  change(method: string, _url: Url, _as: Url, options: any): Promise<boolean> {
+  change(
+    method: HistoryMethod,
+    _url: Url,
+    _as: Url,
+    options: any
+  ): Promise<boolean> {
     return new Promise((resolve, reject) => {
       if (!options._h) {
         this.isSsr = false
@@ -444,28 +451,35 @@ export default class Router implements BaseRouter {
     })
   }
 
-  changeState(method: string, url: string, as: string, options = {}): void {
+  changeState(
+    method: HistoryMethod,
+    url: string,
+    as: string,
+    options = {}
+  ): void {
     if (process.env.NODE_ENV !== 'production') {
       if (typeof window.history === 'undefined') {
         console.error(`Warning: window.history is not available.`)
         return
       }
-      // @ts-ignore method should always exist on history
+
       if (typeof window.history[method] === 'undefined') {
         console.error(`Warning: window.history.${method} is not available`)
         return
       }
     }
 
     if (method !== 'pushState' || getURL() !== as) {
-      // @ts-ignore method should always exist on history
       window.history[method](
         {
           url,
           as,
           options,
         },
-        null,
+        // Most browsers currently ignores this parameter, although they may use it in the future.
+        // Passing the empty string here should be safe against future changes to the method.
+        // https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState
+        '',
         as
       )
     }

diff --git a/packages/next/next-server/server/lib/path-match.ts b/packages/next/next-server/server/lib/path-match.ts
@@ -49,8 +49,7 @@ function decodeParam(param: string) {
   try {
     return decodeURIComponent(param)
   } catch (_) {
-    const err = new Error('failed to decode param')
-    // @ts-ignore DECODE_FAILED is handled
+    const err: Error & { code?: string } = new Error('failed to decode param')
     err.code = 'DECODE_FAILED'
     throw err
   }

diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx
@@ -400,8 +400,8 @@ export async function renderToHTML(
 
   await Loadable.preloadAll() // Make sure all dynamic imports are loaded
 
-  // @ts-ignore url will always be set
-  const asPath: string = req.url
+  // url will always be set
+  const asPath = req.url as string
   const router = new ServerRouter(pathname, query, asPath, {
     isFallback: isFallback,
   })

diff --git a/packages/next/types/misc.d.ts b/packages/next/types/misc.d.ts
@@ -91,7 +91,28 @@ declare module 'autodll-webpack-plugin' {
   export = AutoDllPlugin
 }
 
+declare module 'pnp-webpack-plugin' {
+  import webpack from 'webpack'
+  import ForkTsCheckerWebpackPlugin from 'fork-ts-checker-webpack-plugin'
+
+  class PnpWebpackPlugin extends webpack.Plugin {
+    static forkTsCheckerOptions: <
+      T extends Partial<ForkTsCheckerWebpackPlugin.Options>
+    >(
+      settings: T
+    ) => T & {
+      resolveModuleNameModule?: string
+      resolveTypeReferenceDirectiveModule?: string
+    }
+  }
+
+  export = PnpWebpackPlugin
+}
+
 declare module NodeJS {
+  interface ProcessVersions {
+    pnp?: string
+  }
   interface Process {
     crossOrigin?: string
   }

diff --git a/packages/next/types/webpack.d.ts b/packages/next/types/webpack.d.ts
@@ -56,6 +56,10 @@ declare module 'webpack' {
   ): webpack.MultiWatching | webpack.MultiCompiler
   function webpack(options: webpack.Configuration[]): webpack.MultiCompiler
 
+  function webpack(
+    options: webpack.Configuration | webpack.Configuration[]
+  ): webpack.Compiler | webpack.MultiCompiler
+
   namespace webpack {
     /** Webpack package version. */
     const version: string | undefined