Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change default typescript moduleResolution to bundler to match nextjs heuristics of using webpack/turbopack #75805

Open
wants to merge 2 commits into
base: canary
Choose a base branch
from

Conversation

fuxingloh
Copy link

@fuxingloh fuxingloh commented Feb 8, 2025

Piggybacking on top of #51957
I believe the default generated tsconfig.json module resolution should be bundler for better DevEx and typescript language server compatibility—matching nextjs heuristics of using webpack/turbopack.

          "module": "esnext",
-         "moduleResolution": "node",
+         "moduleResolution": "bundler",
          "noEmit": true,

I started a new NextJS project manually (nextra to be precise) instead of using create-next-app, ran pnpm run dev and got an TS error/warning. The typescript language server was complaining that it doesn't understand the import semantics:

TS2307: Cannot find module nextra/ page-map or its corresponding type declarations.
There are types at
  /Users/fuxingloh/projects/some-docs/node_modules/nextra/dist/server/page-map/index.d.ts
,but this result could not be resolved under your current moduleResolution setting. Consider updating to node16, nodenext, or bundler

I ditched the manual setup and instead used create-next-app to set it up. I saw the tsconfig.json provided in create-next-app was different from the one created by "We detected TypeScript in your project and created a".

@ijjk ijjk added the type: next label Feb 8, 2025
@ijjk
Copy link
Member

ijjk commented Feb 8, 2025

Allow CI Workflow Run

  • approve CI run for commit: 8170ec6

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@fuxingloh fuxingloh changed the title Change default typescript moduleResolution to bundler to match nextjs heuristics of using webpack/turbppack Change default typescript moduleResolution to bundler to match nextjs heuristics of using webpack/turbopack Feb 8, 2025
@fuxingloh fuxingloh marked this pull request as ready for review February 8, 2025 06:45
eps1lon
eps1lon previously approved these changes Feb 9, 2025
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically just completing #51957. Let's see what CI says.

Thank you.

@ijjk
Copy link
Member

ijjk commented Feb 9, 2025

Failing test suites

Commit: 097748a

pnpm test-dev-turbo test/development/correct-tsconfig-defaults/index.test.ts (turbopack)

  • correct tsconfig.json defaults > should add moduleResolution when generating tsconfig.json in dev
  • correct tsconfig.json defaults > should not warn for moduleResolution when already present and valid
Expand output

● correct tsconfig.json defaults › should add moduleResolution when generating tsconfig.json in dev

expect(received).toEqual(expected) // deep equality

- Expected  -  2
+ Received  + 18

- ObjectContaining {
-   "moduleResolution": "node",
+ Object {
+   "allowJs": true,
+   "esModuleInterop": true,
+   "incremental": true,
+   "isolatedModules": true,
+   "jsx": "preserve",
+   "lib": Array [
+     "dom",
+     "dom.iterable",
+     "esnext",
+   ],
+   "module": "esnext",
+   "moduleResolution": "bundler",
+   "noEmit": true,
+   "resolveJsonModule": true,
+   "skipLibCheck": true,
+   "strict": false,
+   "target": "ES2017",
  }

  39 |       expect(next.cliOutput).not.toContain('moduleResolution')
  40 |
> 41 |       expect(tsconfig.compilerOptions).toEqual(
     |                                        ^
  42 |         expect.objectContaining({ moduleResolution: 'node' })
  43 |       )
  44 |     } finally {

  at Object.toEqual (development/correct-tsconfig-defaults/index.test.ts:41:40)

● correct tsconfig.json defaults › should not warn for moduleResolution when already present and valid

expect(received).toEqual(expected) // deep equality

- Expected  -  2
+ Received  + 18

- ObjectContaining {
-   "moduleResolution": "node",
+ Object {
+   "allowJs": true,
+   "esModuleInterop": true,
+   "incremental": true,
+   "isolatedModules": true,
+   "jsx": "preserve",
+   "lib": Array [
+     "dom",
+     "dom.iterable",
+     "esnext",
+   ],
+   "module": "esnext",
+   "moduleResolution": "bundler",
+   "noEmit": true,
+   "resolveJsonModule": true,
+   "skipLibCheck": true,
+   "strict": false,
+   "target": "ES2017",
  }

  57 |       const tsconfig = JSON.parse(await next.readFile('tsconfig.json'))
  58 |
> 59 |       expect(tsconfig.compilerOptions).toEqual(
     |                                        ^
  60 |         expect.objectContaining({ moduleResolution: 'node' })
  61 |       )
  62 |       expect(next.cliOutput).not.toContain('moduleResolution')

  at Object.toEqual (development/correct-tsconfig-defaults/index.test.ts:59:40)

Read more about building and testing Next.js in contributing.md.

pnpm test-start-turbo test/e2e/app-dir/next-dist-client-esm-import/next-dist-client-esm-import.test.ts (turbopack)

  • next-dist-client-esm-import > should resolve ESM modules that have "next/dist/client" in their filename
Expand output

● next-dist-client-esm-import › should resolve ESM modules that have "next/dist/client" in their filename

next build failed with code/signal 1

   98 |           if (code || signal)
   99 |             reject(
> 100 |               new Error(`next build failed with code/signal ${code || signal}`)
      |               ^
  101 |             )
  102 |           else resolve()
  103 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:100:15)

Read more about building and testing Next.js in contributing.md.

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

  • tsconfig.json verifier > Creates a default tsconfig.json when one is missing
  • tsconfig.json verifier > Works with an empty tsconfig.json (docs)
  • tsconfig.json verifier > Updates an existing tsconfig.json without losing comments
  • tsconfig.json verifier > allows you to set commonjs module mode
  • tsconfig.json verifier > allows you to set es2020 module mode
  • tsconfig.json verifier > allows you to set target mode
  • tsconfig.json verifier > allows you to set verbatimModuleSyntax true without adding isolatedModules
  • tsconfig.json verifier > allows you to set verbatimModuleSyntax true via extends without adding isolatedModules
Expand output

● tsconfig.json verifier › Creates a default tsconfig.json when one is missing

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `tsconfig.json verifier Creates a default tsconfig.json when one is missing 1`

- Snapshot  - 1
+ Received  + 1

@@ -11,11 +11,11 @@
      "strict": false,
      "noEmit": true,
      "incremental": true,
      "module": "esnext",
      "esModuleInterop": true,
-     "moduleResolution": "node",
+     "moduleResolution": "bundler",
      "resolveJsonModule": true,
      "isolatedModules": true,
      "jsx": "preserve",
      "plugins": [
        {

  25 |       const { code } = await nextBuild(appDir)
  26 |       expect(code).toBe(0)
> 27 |       expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
     |                                                ^
  28 |         "{
  29 |           "compilerOptions": {
  30 |             "target": "ES2017",

  at Object.toMatchInlineSnapshot (integration/tsconfig-verifier/test/index.test.js:27:48)

● tsconfig.json verifier › Works with an empty tsconfig.json (docs)

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `tsconfig.json verifier Works with an empty tsconfig.json (docs) 1`

- Snapshot  - 1
+ Received  + 1

@@ -11,11 +11,11 @@
      "strict": false,
      "noEmit": true,
      "incremental": true,
      "module": "esnext",
      "esModuleInterop": true,
-     "moduleResolution": "node",
+     "moduleResolution": "bundler",
      "resolveJsonModule": true,
      "isolatedModules": true,
      "jsx": "preserve",
      "plugins": [
        {

  80 |       expect(code).toBe(0)
  81 |
> 82 |       expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
     |                                                ^
  83 |         "{
  84 |           "compilerOptions": {
  85 |             "target": "ES2017",

  at Object.toMatchInlineSnapshot (integration/tsconfig-verifier/test/index.test.js:82:48)

● tsconfig.json verifier › Updates an existing tsconfig.json without losing comments

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `tsconfig.json verifier Updates an existing tsconfig.json without losing comments 1`

- Snapshot  - 1
+ Received  + 1

@@ -16,11 +16,11 @@
      "allowJs": true,
      "skipLibCheck": true,
      "strict": false,
      "noEmit": true,
      "incremental": true,
-     "moduleResolution": "node",
+     "moduleResolution": "bundler",
      "resolveJsonModule": true,
      "isolatedModules": true,
      "jsx": "preserve",
      "plugins": [
        {

  147 |       // Weird comma placement until this issue is resolved:
  148 |       // https://github.com/kaelzhang/node-comment-json/issues/21
> 149 |       expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
      |                                                ^
  150 |         "// top-level comment
  151 |         {
  152 |           // in-object comment 1

  at Object.toMatchInlineSnapshot (integration/tsconfig-verifier/test/index.test.js:149:48)

● tsconfig.json verifier › allows you to set commonjs module mode

command failed with code 1 signal null
 ⚠ Linting is disabled.
   No config file found
   No config file found
   ▲ Next.js 15.2.0-canary.46

   Creating an optimized production build ...
   No config file found
   No config file found
   No config file found
 ✓ Compiled successfully
   Checking validity of types ...

   We detected TypeScript in your project and reconfigured your tsconfig.json file for you. Strict-mode is set to false by default.
   The following suggested values were added to your tsconfig.json. These values can be changed to fit your project's needs:

   	- target was set to ES2017 (For top-level `await`. Note: Next.js only polyfills for the esmodules target.)
   	- lib was set to dom,dom.iterable,esnext
   	- allowJs was set to true
   	- skipLibCheck was set to true
   	- strict was set to false
   	- noEmit was set to true
   	- incremental was set to true
   	- include was set to ['next-env.d.ts', '.next/types/**/*.ts', '**/*.ts', '**/*.tsx']
   	- plugins was updated to add { name: 'next' }
   	- strictNullChecks was set to true
   	- exclude was set to ['node_modules']

   The following mandatory changes were made to your tsconfig.json:

   	- esModuleInterop was set to true (requirement for SWC / babel)
   	- moduleResolution was set to bundler (to match webpack resolution)
   	- resolveJsonModule was set to true (to match webpack resolution)
   	- isolatedModules was set to true (requirement for SWC / Babel)
   	- jsx was set to preserve (next.js implements its own optimized jsx transform)



===== TS errors =====

[Test Mode] Type error: Option 'bundler' can only be used when 'module' is set to 'preserve' or to 'es2015' or later.


===== TS errors =====


Failed to compile.

Type error: Option 'bundler' can only be used when 'module' is set to 'preserve' or to 'es2015' or later.

Next.js build worker exited with code: 1 and signal: null

  312 |       ) {
  313 |         return reject(
> 314 |           new Error(
      |           ^
  315 |             `command failed with code ${code} signal ${signal}\n${mergedStdio}`
  316 |           )
  317 |         )

  at ChildProcess.<anonymous> (lib/next-test-utils.ts:314:11)

● tsconfig.json verifier › allows you to set es2020 module mode

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `tsconfig.json verifier allows you to set es2020 module mode 1`

- Snapshot  - 1
+ Received  + 1

@@ -11,11 +11,11 @@
      "allowJs": true,
      "skipLibCheck": true,
      "strict": false,
      "noEmit": true,
      "incremental": true,
-     "moduleResolution": "node",
+     "moduleResolution": "bundler",
      "resolveJsonModule": true,
      "isolatedModules": true,
      "jsx": "preserve",
      "plugins": [
        {

  259 |       expect(code).toBe(0)
  260 |
> 261 |       expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
      |                                                ^
  262 |         "{
  263 |           "compilerOptions": {
  264 |             "esModuleInterop": true,

  at Object.toMatchInlineSnapshot (integration/tsconfig-verifier/test/index.test.js:261:48)

● tsconfig.json verifier › allows you to set target mode

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `tsconfig.json verifier allows you to set target mode 1`

- Snapshot  - 1
+ Received  + 1

@@ -11,11 +11,11 @@
      "strict": false,
      "noEmit": true,
      "incremental": true,
      "module": "esnext",
      "esModuleInterop": true,
-     "moduleResolution": "node",
+     "moduleResolution": "bundler",
      "resolveJsonModule": true,
      "isolatedModules": true,
      "jsx": "preserve",
      "plugins": [
        {

  424 |       expect(code).toBe(0)
  425 |
> 426 |       expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
      |                                                ^
  427 |               "{
  428 |                 "compilerOptions": {
  429 |                   "target": "es2022",

  at Object.toMatchInlineSnapshot (integration/tsconfig-verifier/test/index.test.js:426:48)

● tsconfig.json verifier › allows you to set verbatimModuleSyntax true without adding isolatedModules

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `tsconfig.json verifier allows you to set verbatimModuleSyntax true without adding isolatedModules 1`

- Snapshot  - 1
+ Received  + 1

@@ -12,11 +12,11 @@
      "strict": false,
      "noEmit": true,
      "incremental": true,
      "module": "esnext",
      "esModuleInterop": true,
-     "moduleResolution": "node",
+     "moduleResolution": "bundler",
      "resolveJsonModule": true,
      "jsx": "preserve",
      "plugins": [
        {
          "name": "next"

  536 |       expect(code).toBe(0)
  537 |
> 538 |       expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
      |                                                ^
  539 |         "{
  540 |           "compilerOptions": {
  541 |             "verbatimModuleSyntax": true,

  at Object.toMatchInlineSnapshot (integration/tsconfig-verifier/test/index.test.js:538:48)

● tsconfig.json verifier › allows you to set verbatimModuleSyntax true via extends without adding isolatedModules

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `tsconfig.json verifier allows you to set verbatimModuleSyntax true via extends without adding isolatedModules 1`

- Snapshot  - 1
+ Received  + 1

@@ -12,11 +12,11 @@
      "strict": false,
      "noEmit": true,
      "incremental": true,
      "module": "esnext",
      "esModuleInterop": true,
-     "moduleResolution": "node",
+     "moduleResolution": "bundler",
      "resolveJsonModule": true,
      "jsx": "preserve",
      "plugins": [
        {
          "name": "next"

  594 |       expect(code).toBe(0)
  595 |
> 596 |       expect(await readFile(tsConfig, 'utf8')).toMatchInlineSnapshot(`
      |                                                ^
  597 |         "{
  598 |           "extends": "./tsconfig.base.json",
  599 |           "compilerOptions": {

  at Object.toMatchInlineSnapshot (integration/tsconfig-verifier/test/index.test.js:596:48)

Read more about building and testing Next.js in contributing.md.

@ijjk
Copy link
Member

ijjk commented Feb 9, 2025

Stats from current PR

Default Build (Increase detected ⚠️)
General
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
buildDuration 19s 17.3s N/A
buildDurationCached 16.2s 13.5s N/A
nodeModulesSize 393 MB 393 MB N/A
nextStartRea..uration (ms) 446ms 447ms N/A
Client Bundles (main, webpack)
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
5306-HASH.js gzip 54.3 kB 54.3 kB N/A
8276.HASH.js gzip 169 B 168 B N/A
8377-HASH.js gzip 5.46 kB 5.46 kB N/A
bccd1874-HASH.js gzip 53 kB 53 kB
framework-HASH.js gzip 57.5 kB 57.5 kB N/A
main-app-HASH.js gzip 245 B 246 B N/A
main-HASH.js gzip 34.6 kB 34.5 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 53 kB 53 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB
Overall change 39.4 kB 39.4 kB
Client Pages
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 193 B 193 B
amp-HASH.js gzip 512 B 510 B N/A
css-HASH.js gzip 343 B 342 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB
edge-ssr-HASH.js gzip 265 B 265 B
head-HASH.js gzip 363 B 362 B N/A
hooks-HASH.js gzip 393 B 392 B N/A
image-HASH.js gzip 4.59 kB 4.58 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.35 kB 2.35 kB N/A
routerDirect..HASH.js gzip 328 B 328 B
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 323 B 326 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.59 kB 3.59 kB
Client Build Manifests
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
_buildManifest.js gzip 748 B 747 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
index.html gzip 522 B 523 B N/A
link.html gzip 538 B 538 B
withRouter.html gzip 518 B 520 B N/A
Overall change 538 B 538 B
Edge SSR bundle Size
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
edge-ssr.js gzip 130 kB 130 kB N/A
page.js gzip 211 kB 211 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
middleware-b..fest.js gzip 676 B 671 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31.3 kB 31.3 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 844 B 844 B
Next Runtimes
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
app-page-exp...dev.js gzip 394 kB 394 kB
app-page-exp..prod.js gzip 132 kB 132 kB
app-page-tur..prod.js gzip 145 kB 145 kB
app-page-tur..prod.js gzip 141 kB 141 kB
app-page.run...dev.js gzip 381 kB 381 kB
app-page.run..prod.js gzip 129 kB 129 kB
app-route-ex...dev.js gzip 39.3 kB 39.3 kB
app-route-ex..prod.js gzip 25.6 kB 25.6 kB
app-route-tu..prod.js gzip 25.6 kB 25.6 kB
app-route-tu..prod.js gzip 25.4 kB 25.4 kB
app-route.ru...dev.js gzip 40.9 kB 40.9 kB
app-route.ru..prod.js gzip 25.4 kB 25.4 kB
dist_client_...dev.js gzip 356 B 356 B
dist_client_...dev.js gzip 349 B 349 B
pages-api-tu..prod.js gzip 9.69 kB 9.69 kB
pages-api.ru...dev.js gzip 11.8 kB 11.8 kB
pages-api.ru..prod.js gzip 9.68 kB 9.68 kB
pages-turbo...prod.js gzip 21.9 kB 21.9 kB
pages.runtim...dev.js gzip 31.5 kB 31.5 kB
pages.runtim..prod.js gzip 21.9 kB 21.9 kB
server.runti..prod.js gzip 60.6 kB 60.6 kB
Overall change 1.67 MB 1.67 MB
build cache Overall increase ⚠️
vercel/next.js canary fuxingloh/next.js patch-module-resolution Change
0.pack gzip 2.11 MB 2.11 MB N/A
index.pack gzip 75.4 kB 76.4 kB ⚠️ +1.02 kB
Overall change 75.4 kB 76.4 kB ⚠️ +1.02 kB
Diff details
Diff for main-HASH.js

Diff too large to display

Commit: 097748a

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you take care of updating the remaining tests?

@ijjk ijjk added the tests label Feb 9, 2025
@fuxingloh
Copy link
Author

Can you take care of updating the remaining tests?

I fix a few more that are obvious to me, but I don't have a proper setup so I'm gonna stop here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants