feat: expanded rootDir tsconfig support, --typescript to use src/ directory by default; expanded docs

[ctjlewis]

Feature

• Implements an existing feature request or RFC: `next build` should respect `rootDir` in tsconfig to prevent redundant type-checking #27520
• Integration tests added
• Documentation added
• Telemetry added. In case of a feature if it's used or not.
• Errors have helpful link attached, see contributing.md
• Make sure the linting passes

Changes

Myself and other users ran into trouble with next build type checking unwanted files (outside of rootDir in my case, more specific like excluding TS files in public/ for others).

rootDir and include tsconfig fields prevent unnecessary type checking

Originally, because next build would throw during type checking for files not included in rootDir, I thought it did not respect the rootDir option - but having reviewed/refactored the code, it clear it actually does, and furthermore does not distinguish between any of the tsconfig fields outside of the few fields it overrides (docs were modified to clarify this in multiple locations).

However, if rootDir is configured without adjusting the include patterns from **/*.ts to src/**/*.ts etc., TypeScript will prioritize the includes field and attempt to check the matched files anyway. This was noted in multiple points, especially in the src/ Directory docs.

This PR also adds docs and a test for this situation, demonstrating how to use the rootDir and includes tsconfig fields to ensure that next build ignores type errors in unchecked files. For the use case along the lines of "I have files in public/ I want to ignore," documentation now clarifies in multiple places that next build will honor tsconfig's exclude field (as well as most others).

src/ configuration by default for create-next-app --typescript

Finally, the TypeScript template is updated to use src/ directory configuration by default to minimize the chances of unnecessary type checking. This will likely be seen as an opinionated change, but adding the --typescript flag was itself an opinionated change which I added in #24655, and I think this is the right design choice.

Footnote

My first step was to work through the runTypeCheck.ts logic, which I refactored and commented. This did not end up being necessary, and these changes can be discarded without affecting any of the main goals of this PR, but given that it makes the code easier to reason about, I would recommend leaving the changes in.

[ctjlewis]

Sorry for the early submit - was actually just trying to submit a PR to my own fork and test the CI there before opening an actual PR.

EDIT: CI seems to be temporarily broken for one test, tried to work around but can't figure it out.

EDIT 2: CI does not seem to be in good shape (Failed to fetch timings data Error: request status: 524).

[ctjlewis]

build CI step keeps throwing Failed to fetch timings data Error: request status: 524, seems nondeterministic, wasn't happening at first.

Gonna shelve this for the night, no idea how to proceed with busted CI.

[ijjk]

Failing test suites

Commit: fc321d9

test/unit/webpack-config-overrides.unit.test.js

Expand output

● Test suite failed to run

/home/runner/work/next.js/next.js/packages/next/dist/build/webpack/config/blocks/css/loaders/global.js: No element indexed by 1

  at ArraySet_at [as at] (../node_modules/jest-config/node_modules/source-map/lib/array-set.js:109:9)
      at Array.map (<anonymous>)
      at run.next (<anonymous>)

[ijjk]

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
buildDuration	16.3s	15s	-1.4s
buildDurationCached	3.5s	3.5s	-63ms
nodeModulesSize	50.1 MB	50.1 MB	⚠️ +4.79 kB

Page Load Tests Overall increase ✓

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.648	2.51	-0.14
/ avg req/sec	943.94	995.92	+51.98
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.513	1.283	-0.23
/error-in-render avg req/sec	1652.75	1948.08	+295.33

Client Bundles (main, webpack, commons)

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
745.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	22.8 kB	22.8 kB	✓
webpack-HASH.js gzip	1.5 kB	1.5 kB	✓
Overall change	66.7 kB	66.7 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
_app-HASH.js gzip	980 B	980 B	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	329 B	329 B	✓
dynamic-HASH.js gzip	2.52 kB	2.52 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	903 B	903 B	✓
image-HASH.js gzip	4.1 kB	4.1 kB	✓
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	1.66 kB	1.66 kB	✓
routerDirect..HASH.js gzip	319 B	319 B	✓
script-HASH.js gzip	387 B	387 B	✓
withRouter-HASH.js gzip	320 B	320 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	12.8 kB	12.8 kB	✓

Client Build Manifests

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
_buildManifest.js gzip	490 B	490 B	✓
Overall change	490 B	490 B	✓

Rendered Page Sizes

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
index.html gzip	530 B	530 B	✓
link.html gzip	542 B	542 B	✓
withRouter.html gzip	524 B	524 B	✓
Overall change	1.6 kB	1.6 kB	✓

---

Webpack 4 Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
buildDuration	12.4s	11.7s	-683ms
buildDurationCached	4.6s	4.7s	⚠️ +88ms
nodeModulesSize	50.1 MB	50.1 MB	⚠️ +4.79 kB

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.53	2.53	✓
/ avg req/sec	988.24	988.19	⚠️ -0.05
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.372	1.382	⚠️ +0.01
/error-in-render avg req/sec	1822.36	1808.49	⚠️ -13.87

Client Bundles (main, webpack, commons)

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
17.HASH.js gzip	185 B	185 B	✓
677f882d2ed8..HASH.js gzip	13.8 kB	13.8 kB	✓
framework.HASH.js gzip	41.9 kB	41.9 kB	✓
main-HASH.js gzip	10.6 kB	10.6 kB	✓
webpack-HASH.js gzip	1.19 kB	1.19 kB	✓
Overall change	67.7 kB	67.7 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
_app-HASH.js gzip	965 B	965 B	✓
_error-HASH.js gzip	3.74 kB	3.74 kB	✓
amp-HASH.js gzip	552 B	552 B	✓
css-HASH.js gzip	333 B	333 B	✓
dynamic-HASH.js gzip	2.71 kB	2.71 kB	✓
head-HASH.js gzip	2.97 kB	2.97 kB	✓
hooks-HASH.js gzip	911 B	911 B	✓
index-HASH.js gzip	231 B	231 B	✓
link-HASH.js gzip	1.64 kB	1.64 kB	✓
routerDirect..HASH.js gzip	298 B	298 B	✓
script-HASH.js gzip	2.94 kB	2.94 kB	✓
withRouter-HASH.js gzip	294 B	294 B	✓
e025d2764813..52f.css gzip	125 B	125 B	✓
Overall change	17.7 kB	17.7 kB	✓

Client Build Manifests

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
_buildManifest.js gzip	499 B	499 B	✓
Overall change	499 B	499 B	✓

Rendered Page Sizes

	vercel/next.js canary	ctjlewis/next.js type-check-update	Change
index.html gzip	578 B	578 B	✓
link.html gzip	591 B	591 B	✓
withRouter.html gzip	571 B	571 B	✓
Overall change	1.74 kB	1.74 kB	✓

Commit: fc321d9

[ctjlewis]

The status of this draft is basically that it's purely experimental, including the TS "src/ by default" principle (which was added as a way to guarantee there would never be redundant type-checking).

I think that change should be retained if the idea of refactoring all templates to use a src/ directory by default is reviewed favorably, which it probably would not be, but I would encourage considering this de novo. Otherwise, tsconfig could be configured to only include pages/** and types/** by default so as to prevent redundant type-checking out-of-the-box without relying on the rootDir field.

In short, thoughts on all of this are needed before a concrete PR can be sent in.

[ijjk]

Gonna close this as stale, we did partially migrate to src dir pattern though for the next package!

[ctjlewis]

Thanks JJ, no worries, sorry for never closing these old PRs!

…

On Wed, Jun 14, 2023 at 10:36 PM JJ Kasper ***@***.***> wrote: Closed #27667 <#27667>. — Reply to this email directly, view it on GitHub <#27667 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMUTFCFHMKWTVDF2CCBBKDXLJ7ITANCNFSM5BL2GVFA> . You are receiving this because you authored the thread.Message ID: ***@***.***>