Skip to content

Commit

Permalink
Stop overriding the user's TS config with defaults during next build (
Browse files Browse the repository at this point in the history
#45670)

## Bug

The `next build` command is silently overriding the user's tsconfig when
it shouldn't be; this results in mismatched behavior between `tsc
--noEmit` and `yarn build` and user confusion.

For example, a configuration option like `"moduleResolution":
"nodenext"`, which is preserved and respected by `next dev`, will be
silently overridden to `"moduleResolution": "node"` during `next build`.

This change:
- Fixes #38854
- (probably fixes) #45452 (I have not verified)
- (probably fixes) #41189 (I have not verified)

## Details

Next has a concept of both _defaults_ and _permitted options_ when
modifying/validating the user's tsconfig. The user's config is only
modified if it does not match the _permitted options_. This means that
if the user has specified a permitted value like `"moduleResolution":
"nodenext"`, it will not be overwritten in the user's config file.

However, there was some logic in `runTypeCheck.ts` that did not
adequately capture this nuance – instead, it spread all of the defaults
into the tsconfig it was building before running typecheck, which meant
that if a user had specified an option that was _permitted_ but
_non-default_, it would be overwritten, silently, during `yarn build`
only.

Because Next is already (1) rewriting the TSconfig in
`writeConfigurationDefaults` when the user's config doesn't line up with
what we're expecting and (2) verifying the user's TSConfig remains
correct (in `verifyTypeScriptSetup`) during a `next build`, I believe
that it is safe to remove this config-steamrolling behavior.

## Documentation / Examples

I believe this is strictly a bugfix; it updates the behavior of `next
build` to conform to the same configuration behavior exhibited by `tsc
--noEmit` and `next dev`. Since this is already the user expectation, it
should not require documentation changes.

---------

Co-authored-by: Shu Ding <g@shud.in>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Feb 15, 2023
1 parent 087aa6e commit af6c26c
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 4 deletions.
2 changes: 1 addition & 1 deletion packages/next/src/lib/typescript/runTypeCheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ export async function runTypeCheck(
const requiredConfig = getRequiredConfiguration(ts)

const options = {
...effectiveConfiguration.options,
...requiredConfig,
...effectiveConfiguration.options,
declarationMap: false,
emitDeclarationOnly: false,
noEmit: true,
Expand Down
6 changes: 3 additions & 3 deletions test/integration/tsconfig-verifier/pages/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// eslint-disable-next-line @typescript-eslint/no-unused-vars
/* eslint-disable @typescript-eslint/no-unused-vars */
const blah: boolean = false
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const blah2 = import('../value').then((r) => r.default)
// @ts-expect-error ignore the import issue here caused by https://github.com/microsoft/TypeScript/issues/49083
const blah2 = import('../value.ts').then((r) => r.default)

export default () => <h3>Hello TypeScript</h3>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import helloWorldString from 'pkg/sub-export'

export default function Page() {
return <p>{helloWorldString}</p>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "pkg",
"version": "1.0.0",
"type": "module",
"exports": {
"./sub-export": "./src/some-file.js"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
declare const _default: 'Hello, world'
export default _default
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 'Hello, world'
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { createNextDescribe, FileRef } from 'e2e-utils'
import { join } from 'path'

// regression test suite for https://github.com/vercel/next.js/issues/38854
createNextDescribe(
'Does not override tsconfig moduleResolution field during build',
{
packageJson: { type: 'module' },
files: {
'tsconfig.json': new FileRef(join(__dirname, 'tsconfig.json')),
pages: new FileRef(join(__dirname, 'pages')),
pkg: new FileRef(join(__dirname, 'pkg')),
},
dependencies: {
typescript: 'latest',
'@types/react': 'latest',
'@types/node': 'latest',
pkg: './pkg',
},
},
({ next }) => {
it('boots and renders without throwing an error', async () => {
await next.render$('/')
})
}
)
20 changes: 20 additions & 0 deletions test/production/supports-module-resolution-nodenext/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"compilerOptions": {
"esModuleInterop": true,
"module": "esnext",
"jsx": "preserve",
"target": "es5",
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
"strict": true,
"forceConsistentCasingInFileNames": true,
"noEmit": true,
"incremental": true,
"moduleResolution": "nodenext",
"resolveJsonModule": true,
"isolatedModules": true
},
"exclude": ["node_modules"],
"include": ["next-env.d.ts", "components", "pages"]
}

0 comments on commit af6c26c

Please sign in to comment.