Implement tslint for core files (#5952)

Fixes #5845 

Implement tslint for core files

**What is this?**
Implements tslint for both next and next-server, but keeps standardjs/eslint for the .js files that are still there, we're gradually migrating to Typescript.

**How does it work?**
Before every commit (pre-commit) we execute the following `tslint` command:
`tslint -c tslint.json 'packages/**/*.ts`

**TSLint Rules**
In order to avoid as much changes as possible I marked some rules as false. This way we can improve the linter but making sure this step will not break things. (see tslint.json)

**Note**
After merging this PR, you'll need to update your dependencies since it adds tslint to package.json

package.json

@@ -33,7 +33,7 @@
       "git add"
     ],
     "*.ts": [
-      "standard --parser typescript-eslint-parser --plugin typescript --fix",
+      "tslint -c tslint.json 'packages/**/*.ts'",
       "git add"
     ],
     "packages/**/bin/*": [
@@ -44,7 +44,7 @@
   "standard": {
     "parser": "babel-eslint",
     "ignore": [
-      "packages/next-server/server/next-server.ts",
+      "packages/**/*.ts",
       "**/*.d.ts",
       "**/node_modules/**",
       "examples/with-ioc/**",
@@ -89,6 +89,8 @@
     "rimraf": "2.6.2",
     "standard": "11.0.1",
     "taskr": "1.1.0",
+    "tslint": "5.12.0",
+    "typescript": "3.1.1",
     "typescript-eslint-parser": "21.0.1",
     "wait-port": "0.2.2",
     "wd": "1.10.3",

packages/next-server/server/get-dynamic-import-bundles.ts

@@ -2,7 +2,7 @@ export type ManifestItem = {
   id: number|string,
   name: string,
   file: string,
-  publicPath: string
+  publicPath: string,
 }
 
 export type Manifest = {[moduleId: string]: ManifestItem[]}

packages/next-server/server/get-page-files.ts

@@ -3,8 +3,8 @@ import { normalizePagePath } from './normalize-page-path'
 export type BuildManifest = {
   devFiles: string[],
   pages: {
-    [page: string]: string[]
+    [page: string]: string[],
-  }
+  },
 }
 
 export function getPageFiles(buildManifest: BuildManifest, page: string): string[] {
@@ -12,6 +12,7 @@ export function getPageFiles (buildManifest: BuildManifest, page: string): strin
   const files = buildManifest.pages[normalizedPage]
 
   if (!files) {
+    // tslint:disable-next-line
     console.warn(`Could not find files for ${normalizedPage} in .next/build-manifest.json`)
     return []
   }

packages/next-server/server/load-components.ts

@@ -9,12 +9,12 @@ function interopDefault (mod: any) {
 export async function loadComponents(distDir: string, buildId: string, pathname: string) {
   const documentPath = join(distDir, SERVER_DIRECTORY, CLIENT_STATIC_FILES_PATH, buildId, 'pages', '_document')
   const appPath = join(distDir, SERVER_DIRECTORY, CLIENT_STATIC_FILES_PATH, buildId, 'pages', '_app')
-  let [buildManifest, reactLoadableManifest, Component, Document, App] = await Promise.all([
+  const [buildManifest, reactLoadableManifest, Component, Document, App] = await Promise.all([
     require(join(distDir, BUILD_MANIFEST)),
     require(join(distDir, REACT_LOADABLE_MANIFEST)),
     interopDefault(requirePage(pathname, distDir)),
     interopDefault(require(documentPath)),
-    interopDefault(require(appPath))
+    interopDefault(require(appPath)),
   ])
 
   return {buildManifest, reactLoadableManifest, Component, Document, App}

packages/next-server/server/next-server.ts

@@ -21,7 +21,7 @@ type ServerConstructor = {
   dir?: string,
   staticMarkup?: boolean,
   quiet?: boolean,
-  conf?: NextConfig
+  conf?: NextConfig,
 }
 
 export default class Server {
@@ -35,7 +35,7 @@ export default class Server {
     buildId: string,
     generateEtags: boolean,
     runtimeConfig?: {[key: string]: any},
-    assetPrefix?: string
+    assetPrefix?: string,
   }
   router: Router
 
@@ -54,7 +54,7 @@ export default class Server {
     this.renderOpts = {
       staticMarkup,
       buildId: this.buildId,
-      generateEtags
+      generateEtags,
     }
 
     // Only the `publicRuntimeConfig` key is exposed to the client side
@@ -66,7 +66,7 @@ export default class Server {
     // Initialize next/config with the environment configuration
     envConfig.setConfig({
       serverRuntimeConfig,
-      publicRuntimeConfig
+      publicRuntimeConfig,
     })
 
     const routes = this.generateRoutes()
@@ -80,6 +80,7 @@ export default class Server {
 
   private logError(...args: any): void {
     if (this.quiet) return
+    // tslint:disable-next-line
     console.error(...args)
   }
 
@@ -136,14 +137,14 @@ export default class Server {
           }
           const p = join(this.distDir, CLIENT_STATIC_FILES_PATH, ...(params.path || []))
           await this.serveStatic(req, res, p, parsedUrl)
-        }
+        },
       },
       {
         match: route('/_next/:path*'),
         // This path is needed because `render()` does a check for `/_next` and the calls the routing again
         fn: async (req, res, _params, parsedUrl) => {
           await this.render404(req, res, parsedUrl)
-        }
+        },
       },
       {
         // It's very important to keep this route's param optional.
@@ -154,8 +155,8 @@ export default class Server {
         fn: async (req, res, params, parsedUrl) => {
           const p = join(this.dir, 'static', ...(params.path || []))
           await this.serveStatic(req, res, p, parsedUrl)
-        }
+        },
-      }
+      },
     ]
 
     if (this.nextConfig.useFileSystemPublicRoutes) {
@@ -171,7 +172,7 @@ export default class Server {
             throw new Error('pathname is undefined')
           }
           await this.render(req, res, pathname, query, parsedUrl)
-        }
+        },
       })
     }
 

packages/next-server/server/require.ts

@@ -15,6 +15,7 @@ export function getPagePath (page: string, distDir: string): string {
   try {
     page = normalizePagePath(page)
   } catch (err) {
+    // tslint:disable-next-line
     console.error(err)
     throw pageNotFoundError(page)
   }

packages/next-server/server/router.ts

@@ -8,7 +8,7 @@ type Params = {[param: string]: string}
 
 export type Route = {
   match: (pathname: string|undefined) => false|Params,
-  fn: (req: IncomingMessage, res: ServerResponse, params: Params, parsedUrl: UrlWithParsedQuery) => void
+  fn: (req: IncomingMessage, res: ServerResponse, params: Params, parsedUrl: UrlWithParsedQuery) => void,
 }
 
 export default class Router {

packages/next-server/server/utils.ts

@@ -2,7 +2,7 @@ import { BLOCKED_PAGES } from 'next-server/constants'
 
 const internalPrefixes = [
   /^\/_next\//,
-  /^\/static\//
+  /^\/static\//,
 ]
 
 export function isInternalUrl(url: string): boolean {

packages/next/bin/next-build.ts

@@ -9,7 +9,7 @@ const args = arg({
   // Types
   '--help': Boolean,
   // Aliases
-  '-h': '--help'
+  '-h': '--help',
 })
 
 if (args['--help']) {
@@ -45,6 +45,7 @@ if (!existsSync(join(dir, 'pages'))) {
 
 build(dir)
   .catch((err) => {
+    // tslint:disable-next-line
     console.error('> Build error occurred')
     printAndExit(err)
   })

packages/next/bin/next-dev.ts

@@ -14,10 +14,11 @@ const args = arg({
   // Aliases
   '-h': '--help',
   '-p': '--port',
-  '-H': '--hostname'
+  '-H': '--hostname',
 })
 
 if (args['--help']) {
+  // tslint:disable-next-line
   console.log(`
     Description
       Starts the application in development mode (hot-code reloading, error
@@ -56,6 +57,7 @@ if (!existsSync(join(dir, 'pages'))) {
 const port = args['--port'] || 3000
 startServer({dir, dev: true}, port, args['--hostname'])
   .then(async (app) => {
+    // tslint:disable-next-line
     console.log(`> Ready on http://${args['--hostname'] || 'localhost'}:${port}`)
     await app.prepare()
   })
@@ -63,18 +65,19 @@ startServer({dir, dev: true}, port, args['--hostname'])
     if (err.code === 'EADDRINUSE') {
       let errorMessage = `Port ${port} is already in use.`
       const pkgAppPath = require('find-up').sync('package.json', {
-        cwd: dir
+        cwd: dir,
       })
       const appPackage = require(pkgAppPath)
       if (appPackage.scripts) {
-        const nextScript = Object.entries(appPackage.scripts).find(scriptLine => scriptLine[1] === 'next')
+        const nextScript = Object.entries(appPackage.scripts).find((scriptLine) => scriptLine[1] === 'next')
         if (nextScript) {
           errorMessage += `\nUse \`npm run ${nextScript[0]} -- -p <some other port>\`.`
         }
       }
-
+      // tslint:disable-next-line
       console.error(errorMessage)
     } else {
+      // tslint:disable-next-line
       console.error(err)
     }
     process.nextTick(() => process.exit(1))

packages/next/bin/next-export.ts

@@ -16,10 +16,11 @@ const args = arg({
   // Aliases
   '-h': '--help',
   '-s': '--silent',
-  '-o': '--outdir'
+  '-o': '--outdir',
 })
 
 if (args['--help']) {
+  // tslint:disable-next-line
   console.log(`
     Description
       Exports the application for production deployment
@@ -57,7 +58,7 @@ const options = {
   silent: args['--silent'] || false,
   threads: args['--threads'],
   concurrency: args['--concurrency'],
-  outdir: args['--outdir'] ? resolve(args['--outdir']) : join(dir, 'out')
+  outdir: args['--outdir'] ? resolve(args['--outdir']) : join(dir, 'out'),
 }
 
 exportApp(dir, options)

packages/next/bin/next-start.ts

@@ -13,10 +13,11 @@ const args = arg({
   // Aliases
   '-h': '--help',
   '-p': '--port',
-  '-H': '--hostname'
+  '-H': '--hostname',
 })
 
 if (args['--help']) {
+  // tslint:disable-next-line
   console.log(`
     Description
       Starts the application in production mode.
@@ -42,10 +43,12 @@ const dir = resolve(args._[0] || '.')
 const port = args['--port'] || 3000
 startServer({dir}, port, args['--hostname'])
   .then(async (app) => {
+    // tslint:disable-next-line
     console.log(`> Ready on http://${args['--hostname']}:${port}`)
     await app.prepare()
   })
   .catch((err) => {
+    // tslint:disable-next-line
     console.error(err)
     process.exit(1)
   })

packages/next/bin/next.ts

@@ -3,11 +3,12 @@ import { join } from 'path'
 import spawn from 'cross-spawn'
 import arg from 'arg'
 
-['react', 'react-dom'].forEach(dependency => {
+['react', 'react-dom'].forEach((dependency) => {
   try {
     // When 'npm link' is used it checks the clone location. Not the project.
     require.resolve(dependency)
   } catch (err) {
+    // tslint:disable-next-line
     console.warn(`The module '${dependency}' was not found. Next.js requires that you include it in 'dependencies' of your 'package.json'. To add it, run 'npm install --save ${dependency}'`)
   }
 })
@@ -17,7 +18,7 @@ const commands = [
   'build',
   'start',
   'export',
-  defaultCommand
+  defaultCommand,
 ]
 
 const args = arg({
@@ -28,23 +29,25 @@ const args = arg({
 
   // Aliases
   '-v': '--version',
-  '-h': '--help'
+  '-h': '--help',
 }, {
-  permissive: true
+  permissive: true,
 })
 
 // Version is inlined into the file using taskr build pipeline
 if (args['--version']) {
+  // tslint:disable-next-line
   console.log(`Next.js v${process.env.NEXT_VERSION}`)
   process.exit(0)
 }
 
 // Check if we are running `next <subcommand>` or `next`
-const foundCommand = args._.find(cmd => commands.includes(cmd))
+const foundCommand = args._.find((cmd) => commands.includes(cmd))
 
 // Makes sure the `next <subcommand> --help` case is covered
 // This help message is only showed for `next --help`
 if (!foundCommand && args['--help']) {
+  // tslint:disable-next-line
   console.log(`
     Usage
       $ next <command>
@@ -65,12 +68,13 @@ if (!foundCommand && args['--help']) {
 
 const nodeArguments: string[] = []
 if (args['--inspect']) {
+  // tslint:disable-next-line
   console.log('Passing "--inspect" to Node.js')
   nodeArguments.push('--inspect')
 }
 
 const command = foundCommand || defaultCommand
-const forwardedArgs = args._.filter(arg => arg !== command)
+const forwardedArgs = args._.filter((arg) => arg !== command)
 
 // Make sure the `next <subcommand> --help` case is covered
 if (args['--help']) {
@@ -92,12 +96,14 @@ const startProcess = () => {
       if (signal === 'SIGKILL') {
         process.exit(137)
       }
+      // tslint:disable-next-line
       console.log(`got signal ${signal}, exiting`)
       process.exit(signal === 'SIGINT' ? 0 : 1)
     }
     process.exit(0)
   })
   proc.on('error', (err) => {
+    // tslint:disable-next-line
     console.error(err)
     process.exit(1)
   })
@@ -121,6 +127,7 @@ if (command === 'dev') {
   const {watchFile} = require('fs')
   watchFile(`${process.cwd()}/${CONFIG_FILE}`, (cur: any, prev: any) => {
     if (cur.size > 0 || prev.size > 0) {
+      // tslint:disable-next-line
       console.log(`\n> Found a change in ${CONFIG_FILE}, restarting the server...`)
       // Don't listen to 'close' now since otherwise parent gets killed by listener
       proc.removeAllListeners('close')

tslint.json

@@ -0,0 +1,30 @@
+{
+  "defaultSeverity": "error",
+  "extends": [
+    "tslint:recommended"
+  ],
+  "jsRules": {},
+  "rules": {
+    "curly": false,
+    "no-empty": false,
+    "object-literal-sort-keys": false,
+    "variable-name": false,
+    "max-line-length": false,
+    "interface-over-type-literal": false,
+    "no-shadowed-variable": false,
+    "no-var-requires": false,
+    "semicolon": false,
+    "quotemark": false,
+    "ordered-imports": false,
+    "member-access": false,
+    "member-ordering": false
+  },
+  "rulesDirectory": [],
+  "linterOptions": {
+    "exclude": [
+      "**/node_modules/**",
+      "**/build/**",
+      "**/types/**"
+    ]
+  }
+}