Do not throw away `tsconfig.json` comments (#13458)

This pull request updates our TypeScript verification process to not wipe out potentially vital user comments.

Introducing a prompt process was mostly a side effect of users wanting to keep comments.
There's no reason we really need this prompt, as answering no would refuse to boot the Next.js server anyway.

---

Fixes #8128
Closes #11440

packages/next/compiled/comment-json/LICENSE

+Copyright (c) 2013 kaelzhang <>, contributors
+http://kael.me/
+
+Permission is hereby granted, free of charge, to any person obtaining
+a copy of this software and associated documentation files (the
+"Software"), to deal in the Software without restriction, including
+without limitation the rights to use, copy, modify, merge, publish,
+distribute, sublicense, and/or sell copies of the Software, and to
+permit persons to whom the Software is furnished to do so, subject to
+the following conditions:
+
+The above copyright notice and this permission notice shall be
+included in all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

packages/next/compiled/comment-json/package.json

+{"name":"comment-json","main":"index.js","author":"kaelzhang","license":"MIT"}

packages/next/lib/verifyTypeScriptSetup.ts

+import { promises as fsPromises } from 'fs'
+import { writeFile } from 'fs-extra'
 import chalk from 'next/dist/compiled/chalk'
-import fs from 'fs'
+import * as CommentJson from 'next/dist/compiled/comment-json'
 import os from 'os'
 import path from 'path'
-
 import { fileExists } from './file-exists'
 import { recursiveReadDir } from './recursive-readdir'
 import { resolveRequest } from './resolve-request'
 
-function writeJson(fileName: string, object: object): Promise<void> {
-  return fs.promises.writeFile(
-    fileName,
-    JSON.stringify(object, null, 2).replace(/\n/g, os.EOL) + os.EOL
-  )
-}
-
 async function hasTypeScript(dir: string): Promise<boolean> {
   const typescriptFiles = await recursiveReadDir(
     dir,
@@ -105,7 +99,7 @@ export async function verifyTypeScriptSetup(
 
   let firstTimeSetup = false
   if (hasTsConfig) {
-    const tsConfig = await fs.promises
+    const tsConfig = await fsPromises
       .readFile(tsConfigPath, 'utf8')
       .then((val) => val.trim())
     firstTimeSetup = tsConfig === '' || tsConfig === '{}'
@@ -176,13 +170,11 @@ export async function verifyTypeScriptSetup(
     )
     console.log()
 
-    await writeJson(tsConfigPath, {})
+    await writeFile(tsConfigPath, '{}' + os.EOL)
   }
 
-  const messages = []
-  let appTsConfig
-  let parsedTsConfig
-  let parsedCompilerOptions
+  let resolvedTsConfig
+  let resolvedCompilerOptions
   try {
     const { config: readTsConfig, error } = ts.readConfigFile(
       tsConfigPath,
@@ -193,14 +185,14 @@ export async function verifyTypeScriptSetup(
       throw new Error(ts.formatDiagnostic(error, formatDiagnosticHost))
     }
 
-    appTsConfig = readTsConfig
+    resolvedTsConfig = readTsConfig
 
     // Get TS to parse and resolve any "extends"
     // Calling this function also mutates the tsconfig, adding in "include" and
     // "exclude", but the compilerOptions remain untouched
-    parsedTsConfig = JSON.parse(JSON.stringify(readTsConfig))
+    const throwAwayConfig = JSON.parse(JSON.stringify(readTsConfig))
     const result = ts.parseJsonConfigFileContent(
-      parsedTsConfig,
+      throwAwayConfig,
       ts.sys,
       path.dirname(tsConfigPath)
     )
@@ -219,7 +211,7 @@ export async function verifyTypeScriptSetup(
       )
     }
 
-    parsedCompilerOptions = result.options
+    resolvedCompilerOptions = result.options
   } catch (e) {
     if (e && e.name === 'SyntaxError') {
       console.error(
@@ -236,11 +228,16 @@ export async function verifyTypeScriptSetup(
     return
   }
 
-  if (appTsConfig.compilerOptions == null) {
-    appTsConfig.compilerOptions = {}
+  const userTsConfigContent = await fsPromises.readFile(tsConfigPath, {
+    encoding: 'utf8',
+  })
+  const userTsConfig = CommentJson.parse(userTsConfigContent)
+  if (userTsConfig.compilerOptions == null) {
+    userTsConfig.compilerOptions = {}
     firstTimeSetup = true
   }
 
+  const messages = []
   for (const option of Object.keys(compilerOptions)) {
     const { parsedValue, value, suggested, reason } = compilerOptions[option]
 
@@ -248,16 +245,16 @@ export async function verifyTypeScriptSetup(
     const coloredOption = chalk.cyan('compilerOptions.' + option)
 
     if (suggested != null) {
-      if (parsedCompilerOptions[option] === undefined) {
-        appTsConfig.compilerOptions[option] = suggested
+      if (resolvedCompilerOptions[option] === undefined) {
+        userTsConfig.compilerOptions[option] = suggested
         messages.push(
           `${coloredOption} to be ${chalk.bold(
             'suggested'
           )} value: ${chalk.cyan.bold(suggested)} (this can be changed)`
         )
       }
-    } else if (parsedCompilerOptions[option] !== valueToCheck) {
-      appTsConfig.compilerOptions[option] = value
+    } else if (resolvedCompilerOptions[option] !== valueToCheck) {
+      userTsConfig.compilerOptions[option] = value
       messages.push(
         `${coloredOption} ${chalk.bold(
           valueToCheck == null ? 'must not' : 'must'
@@ -268,12 +265,12 @@ export async function verifyTypeScriptSetup(
   }
 
   // tsconfig will have the merged "include" and "exclude" by this point
-  if (parsedTsConfig.exclude == null) {
-    appTsConfig.exclude = ['node_modules']
+  if (resolvedTsConfig.exclude == null) {
+    userTsConfig.exclude = ['node_modules']
   }
 
-  if (parsedTsConfig.include == null) {
-    appTsConfig.include = ['next-env.d.ts', '**/*.ts', '**/*.tsx']
+  if (resolvedTsConfig.include == null) {
+    userTsConfig.include = ['next-env.d.ts', '**/*.ts', '**/*.tsx']
   }
 
   if (messages.length > 0) {
@@ -299,13 +296,17 @@ export async function verifyTypeScriptSetup(
       })
       console.warn()
     }
-    await writeJson(tsConfigPath, appTsConfig)
+    await fsPromises.writeFile(
+      tsConfigPath,
+      CommentJson.stringify(userTsConfig, null, 2) + os.EOL
+    )
   }
 
   // Reference `next` types
   const appTypeDeclarations = path.join(dir, 'next-env.d.ts')
-  if (!fs.existsSync(appTypeDeclarations)) {
-    fs.writeFileSync(
+  const hasAppTypeDeclarations = await fileExists(appTypeDeclarations)
+  if (!hasAppTypeDeclarations) {
+    await fsPromises.writeFile(
       appTypeDeclarations,
       '/// <reference types="next" />' +
         os.EOL +

packages/next/package.json

@@ -126,6 +126,7 @@
     "@types/babel__template": "7.0.2",
     "@types/babel__traverse": "7.0.10",
     "@types/ci-info": "2.0.0",
+    "@types/comment-json": "1.1.1",
     "@types/compression": "0.0.36",
     "@types/content-type": "1.1.3",
     "@types/cookie": "0.3.3",
@@ -166,6 +167,7 @@
     "cache-loader": "4.1.0",
     "chalk": "2.4.2",
     "ci-info": "watson/ci-info#f43f6a1cefff47fb361c88cf4b943fdbcaafe540",
+    "comment-json": "3.0.2",
     "compression": "1.7.4",
     "conf": "5.0.0",
     "content-type": "1.0.4",

packages/next/taskfile.js

@@ -517,6 +517,14 @@ export async function ncc_terser_webpack_plugin(task, opts) {
     .target('compiled/terser-webpack-plugin')
 }
 
+externals['comment-json'] = 'next/dist/compiled/comment-json'
+export async function ncc_comment_json(task, opts) {
+  await task
+    .source(opts.src || relative(__dirname, require.resolve('comment-json')))
+    .ncc({ packageName: 'comment-json', externals })
+    .target('compiled/comment-json')
+}
+
 externals['path-to-regexp'] = 'next/dist/compiled/path-to-regexp'
 export async function path_to_regexp(task, opts) {
   await task
@@ -592,6 +600,7 @@ export async function ncc(task) {
       'ncc_webpack_dev_middleware',
       'ncc_webpack_hot_middleware',
       'ncc_terser_webpack_plugin',
+      'ncc_comment_json',
     ])
 }
 

packages/next/types/misc.d.ts

@@ -220,6 +220,10 @@ declare module 'next/dist/compiled/terser-webpack-plugin' {
   import m from 'terser-webpack-plugin'
   export = m
 }
+declare module 'next/dist/compiled/comment-json' {
+  import m from 'comment-json'
+  export = m
+}
 
 declare module 'autodll-webpack-plugin' {
   import webpack from 'webpack'

test/integration/tsconfig-verifier/test/index.test.js

 /* eslint-env jest */
 
+import { writeFile } from 'fs'
+import { createFile, exists, readFile, remove } from 'fs-extra'
+import { nextBuild } from 'next-test-utils'
 import path from 'path'
-import {
-  exists,
-  remove,
-  readFile,
-  readJSON,
-  writeJSON,
-  createFile,
-} from 'fs-extra'
-
-import { launchApp, findPort, killApp, renderViaHTTP } from 'next-test-utils'
 
 jest.setTimeout(1000 * 60 * 5)
 
-describe('Fork ts checker plugin', () => {
+describe('tsconfig.json verifier', () => {
   const appDir = path.join(__dirname, '../')
   const tsConfig = path.join(appDir, 'tsconfig.json')
-  let appPort
-  let app
 
-  beforeAll(async () => {
-    appPort = await findPort()
-    app = await launchApp(appDir, appPort)
+  beforeEach(async () => {
+    await remove(tsConfig)
   })
 
-  afterAll(async () => {
-    await killApp(app)
+  afterEach(async () => {
     await remove(tsConfig)
   })
 
   it('Creates a default tsconfig.json when one is missing', async () => {
-    expect(await exists(tsConfig)).toBe(true)
-
-    const tsConfigContent = await readFile(tsConfig)
-    let parsedTsConfig
-    expect(() => {
-      parsedTsConfig = JSON.parse(tsConfigContent)
-    }).not.toThrow()
-
-    expect(parsedTsConfig.exclude[0]).toBe('node_modules')
+    expect(await exists(tsConfig)).toBe(false)
+    const { code } = await nextBuild(appDir)
+    expect(code).toBe(0)
+    expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
+      "{
+        \\"compilerOptions\\": {
+          \\"target\\": \\"es5\\",
+          \\"lib\\": [
+            \\"dom\\",
+            \\"dom.iterable\\",
+            \\"esnext\\"
+          ],
+          \\"allowJs\\": true,
+          \\"skipLibCheck\\": true,
+          \\"strict\\": false,
+          \\"forceConsistentCasingInFileNames\\": true,
+          \\"noEmit\\": true,
+          \\"esModuleInterop\\": true,
+          \\"module\\": \\"esnext\\",
+          \\"moduleResolution\\": \\"node\\",
+          \\"resolveJsonModule\\": true,
+          \\"isolatedModules\\": true,
+          \\"jsx\\": \\"preserve\\"
+        },
+        \\"exclude\\": [
+          \\"node_modules\\"
+        ],
+        \\"include\\": [
+          \\"next-env.d.ts\\",
+          \\"**/*.ts\\",
+          \\"**/*.tsx\\"
+        ]
+      }
+      "
+    `)
   })
 
   it('Works with an empty tsconfig.json (docs)', async () => {
-    await killApp(app)
-
-    await remove(tsConfig)
-    await new Promise((resolve) => setTimeout(resolve, 500))
-
     expect(await exists(tsConfig)).toBe(false)
 
     await createFile(tsConfig)
     await new Promise((resolve) => setTimeout(resolve, 500))
-
     expect(await readFile(tsConfig, 'utf8')).toBe('')
 
-    await killApp(app)
-    await new Promise((resolve) => setTimeout(resolve, 500))
-
-    appPort = await findPort()
-    app = await launchApp(appDir, appPort)
-
-    const tsConfigContent = await readFile(tsConfig)
-    let parsedTsConfig
-    expect(() => {
-      parsedTsConfig = JSON.parse(tsConfigContent)
-    }).not.toThrow()
-
-    expect(parsedTsConfig.exclude[0]).toBe('node_modules')
+    const { code } = await nextBuild(appDir)
+    expect(code).toBe(0)
+
+    expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
+      "{
+        \\"compilerOptions\\": {
+          \\"target\\": \\"es5\\",
+          \\"lib\\": [
+            \\"dom\\",
+            \\"dom.iterable\\",
+            \\"esnext\\"
+          ],
+          \\"allowJs\\": true,
+          \\"skipLibCheck\\": true,
+          \\"strict\\": false,
+          \\"forceConsistentCasingInFileNames\\": true,
+          \\"noEmit\\": true,
+          \\"esModuleInterop\\": true,
+          \\"module\\": \\"esnext\\",
+          \\"moduleResolution\\": \\"node\\",
+          \\"resolveJsonModule\\": true,
+          \\"isolatedModules\\": true,
+          \\"jsx\\": \\"preserve\\"
+        },
+        \\"exclude\\": [
+          \\"node_modules\\"
+        ],
+        \\"include\\": [
+          \\"next-env.d.ts\\",
+          \\"**/*.ts\\",
+          \\"**/*.tsx\\"
+        ]
+      }
+      "
+    `)
   })
 
-  it('Updates an existing tsconfig.json with required value', async () => {
-    await killApp(app)
-
-    let parsedTsConfig = await readJSON(tsConfig)
-    parsedTsConfig.compilerOptions.esModuleInterop = false
-
-    await writeJSON(tsConfig, parsedTsConfig)
-    appPort = await findPort()
-    app = await launchApp(appDir, appPort)
-
-    parsedTsConfig = await readJSON(tsConfig)
-    expect(parsedTsConfig.compilerOptions.esModuleInterop).toBe(true)
-  })
+  it('Updates an existing tsconfig.json without losing comments', async () => {
+    expect(await exists(tsConfig)).toBe(false)
 
-  it('Renders a TypeScript page correctly', async () => {
-    const html = await renderViaHTTP(appPort, '/')
-    expect(html).toMatch(/Hello TypeScript/)
+    await writeFile(
+      tsConfig,
+      `
+      // top-level comment
+      {
+        // in-object comment 1
+        "compilerOptions": {
+          // in-object comment
+          "esModuleInterop": false, // this should be true
+          "module": "commonjs" // should not be commonjs
+          // end-object comment
+        }
+        // in-object comment 2
+      }
+      // end comment
+      `
+    )
+    await new Promise((resolve) => setTimeout(resolve, 500))
+    const { code } = await nextBuild(appDir)
+    expect(code).toBe(0)
+
+    // Weird comma placement until this issue is resolved:
+    // https://github.com/kaelzhang/node-comment-json/issues/21
+    expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
+      "// top-level comment
+      {
+        // in-object comment 1
+        \\"compilerOptions\\": {
+          // in-object comment
+          \\"esModuleInterop\\": true, // this should be true
+          \\"module\\": \\"esnext\\" // should not be commonjs
+          // end-object comment
+          ,
+          \\"target\\": \\"es5\\",
+          \\"lib\\": [
+            \\"dom\\",
+            \\"dom.iterable\\",
+            \\"esnext\\"
+          ],
+          \\"allowJs\\": true,
+          \\"skipLibCheck\\": true,
+          \\"strict\\": false,
+          \\"forceConsistentCasingInFileNames\\": true,
+          \\"noEmit\\": true,
+          \\"moduleResolution\\": \\"node\\",
+          \\"resolveJsonModule\\": true,
+          \\"isolatedModules\\": true,
+          \\"jsx\\": \\"preserve\\"
+        }
+        // in-object comment 2
+        ,
+        \\"exclude\\": [
+          \\"node_modules\\"
+        ],
+        \\"include\\": [
+          \\"next-env.d.ts\\",
+          \\"**/*.ts\\",
+          \\"**/*.tsx\\"
+        ]
+      }
+      // end comment
+      "
+    `)
   })
 })

yarn.lock

@@ -2493,6 +2493,11 @@
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/@types/color-name/-/color-name-1.1.1.tgz#1c1261bbeaa10a8055bbc5d8ab84b7b2afc846a0"
 
+"@types/comment-json@1.1.1":
+  version "1.1.1"
+  resolved "https://registry.yarnpkg.com/@types/comment-json/-/comment-json-1.1.1.tgz#b4ae889912a93e64619f97989aecaff8ce889dca"
+  integrity sha512-U70oEqvnkeSSp8BIJwJclERtT13rd9ejK7XkIzMCQQePZe3VW1b7iQggXyW4ZvfGtGeXD0pZw24q5iWNe++HqQ==
+
 "@types/compression@0.0.36":
   version "0.0.36"
   resolved "https://registry.yarnpkg.com/@types/compression/-/compression-0.0.36.tgz#7646602ffbfc43ea48a8bf0b2f1d5e5f9d75c0d0"
@@ -4868,6 +4873,16 @@ commander@^5.0.0:
   version "5.1.0"
   resolved "https://registry.yarnpkg.com/commander/-/commander-5.1.0.tgz#46abbd1652f8e059bddaef99bbdcb2ad9cf179ae"
 
+comment-json@3.0.2:
+  version "3.0.2"
+  resolved "https://registry.yarnpkg.com/comment-json/-/comment-json-3.0.2.tgz#a5652a491910e338080bcbf98fc9a37cbd7f3733"
+  integrity sha512-ysJasbJ671+8mPEmwLOfLFqxoGtSmjyoep+lKRVH4J1/hsGu79fwetMDQWk8de8mVgqDZ43D7JuJAlACqjI1pg==
+  dependencies:
+    core-util-is "^1.0.2"
+    esprima "^4.0.1"
+    has-own-prop "^2.0.0"
+    repeat-string "^1.6.1"
+
 commondir@^1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/commondir/-/commondir-1.0.1.tgz#ddd800da0c66127393cca5950ea968a3aaf1253b"
@@ -5138,7 +5153,7 @@ core-js@^2.6.5:
   version "2.6.11"
   resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.6.11.tgz#38831469f9922bded8ee21c9dc46985e0399308c"
 
-core-util-is@1.0.2, core-util-is@~1.0.0:
+core-util-is@1.0.2, core-util-is@^1.0.2, core-util-is@~1.0.0:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.2.tgz#b5fd54220aa2bc5ab57aab7140c940754503c1a7"
 
@@ -7659,6 +7674,11 @@ has-flag@^4.0.0:
   version "4.0.0"
   resolved "https://registry.yarnpkg.com/has-flag/-/has-flag-4.0.0.tgz#944771fd9c81c81265c4d6941860da06bb59479b"
 
+has-own-prop@^2.0.0:
+  version "2.0.0"
+  resolved "https://registry.yarnpkg.com/has-own-prop/-/has-own-prop-2.0.0.tgz#f0f95d58f65804f5d218db32563bb85b8e0417af"
+  integrity sha512-Pq0h+hvsVm6dDEa8x82GnLSYHOzNDt7f0ddFa3FqcQlgzEiptPqL+XrOJNavjOzSYiYWIrgeVYYgGlLmnxwilQ==
+
 has-symbol-support-x@^1.4.1:
   version "1.4.2"
   resolved "https://registry.yarnpkg.com/has-symbol-support-x/-/has-symbol-support-x-1.4.2.tgz#1409f98bc00247da45da67cee0a36f282ff26455"