Skip to content

fix: add a default "include": [] array to generated tsconfig.json, and warn if not populated by user config - #16593

Closed
Rich-Harris wants to merge 3 commits into
version-3from
tsconfig-include-empty
Closed

fix: add a default "include": [] array to generated tsconfig.json, and warn if not populated by user config#16593
Rich-Harris wants to merge 3 commits into
version-3from
tsconfig-include-empty

Conversation

@Rich-Harris

Copy link
Copy Markdown
Member

closes #16581


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

@pkg-svelte-dev

pkg-svelte-dev Bot commented Jul 30, 2026

Copy link
Copy Markdown

Install the latest version of @sveltejs/kit from 150c8a1:

pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/150c8a11e313b94ad00cbf86d501aab20d50bf54

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/kit/pr/16593

@changeset-bot

changeset-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 150c8a1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@svelte-docs-bot

Copy link
Copy Markdown

const resolved = ts.parseJsonConfigFileContent(user_config.options, ts.sys, dir);
const warnings = validate_resolved_config(resolved.options, config.compilerOptions);

if (resolved.raw.include?.length === 0) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If it's completely missing, this would evaluate to undefined, which doesn't equal 0 - right? Should we just have !resolved.raw.include?.length?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or is this going to be inheriting from the generated one, which has []? In which case, why the ?.? Is it just because the types can't know that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's intentional — if include is unspecified, everything inside the directory containing the tsconfig.json is included. if it's specified-but-empty, as is the case for a user config that extends $app/tsconfig, nothing will be included. i guess this could be clearer

Suggested change
if (resolved.raw.include?.length === 0) {
// `$app/tsconfig` specifies `include: []` — if the user config doesn't override this,
// nothing will be included. `$app/tsconfig/service-worker` doesn't specify `include`,
// which is fine — everything in `src/service-worker` is included by default.
// this warns on `[]`, but ignores `undefined`
if (resolved.raw.include?.length === 0) {

const warnings = validate_resolved_config(resolved.options, config.compilerOptions);

if (resolved.raw.include?.length === 0) {
warnings.push(`Missing "include" array`);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This message is not providing enough context and needs actionable advice.

And why do we not do src, test ourselves?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be because the user should specify those themselves in the root tsconfig.json. If we did it for them, the moment they add a new entry to their root include, it overrides what we've written. Better that they specify everything themselves and adjust the one include array

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Exactly — it's a lot more visible if it's in their own config. Ideally the CLI would put src and test and *.ts in automatically.

What about this?

Suggested change
warnings.push(`Missing "include" array`);
warnings.push(`Missing "include" array. Consider adding \`"include": ["src", "test", "*"]\``);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Or more verbosely:

Suggested change
warnings.push(`Missing "include" array`);
warnings.push(`Missing "include" array, which means nothing will be typechecked. Consider adding \`"include": ["src", "test", "*"]\``);

(Trying to avoid making things too verbose, because the text wraps in an ugly awkward way if it takes multiple lines)

@teemingc teemingc Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The first is better because technically everything will get type checked when it's missing. Like if you did a build it would type check your build output files. Maybe we can swap the "*" with a "your-directory" or "your-file"

@teemingc teemingc added needs-ecosystem-ci This PR needs to have an ecosystem CI run started prior to its last commit before merging. and removed needs-ecosystem-ci This PR needs to have an ecosystem CI run started prior to its last commit before merging. labels Aug 4, 2026
Rich-Harris added a commit that referenced this pull request Aug 4, 2026
extracting the test `tsconfig.json` changes out of #16593 so I can put
up an alternative PR without duplicating all that noise
@Rich-Harris

Copy link
Copy Markdown
Member Author

gonna close this in favour of #16645, which I think is a better solution

@Rich-Harris Rich-Harris closed this Aug 4, 2026
Rich-Harris added a commit that referenced this pull request Aug 5, 2026
closes #16581. Different approach to #16593 — rather than adding an
empty `include` array from the generated `tsconfig.json`, we just get
rid of the `exclude` array and validate the result. This removes a layer
of magic and makes it easier for the user to understand the shape of
their config.

---

### Please don't delete this checklist! Before submitting the PR, please
make sure you do the following:
- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.

### Tests
- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint` and `pnpm check`

### Changesets
- [x] If your PR makes a change that should be noted in one or more
packages' changelogs, generate a changeset by running `pnpm changeset`
and following the prompts. Changesets that add features should be
`minor` and those that fix bugs should be `patch`. Please prefix
changeset messages with `feat:`, `fix:`, or `chore:`.

### Edits

- [x] Please ensure that 'Allow edits from maintainers' is checked. PRs
without this option may be closed.

---------

Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Nic Polumeyv <nicolas.polum@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tsconfig exclude node_modules

4 participants