feat: respect tsconfig excludes when using swc#3276
feat: respect tsconfig excludes when using swc#3276enduity wants to merge 6 commits intonestjs:masterfrom
Conversation
8709bf7 to
f1107f1
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for respecting the exclude field in tsconfig.json (or tsconfig.build.json) when using the SWC builder for nest build. Previously, files matching the exclude globs (e.g., **/*.spec.ts) would still appear in the build output when using SWC, while they were correctly excluded when using the default tsc builder.
Changes:
TsConfigProviderOutputtype added totsconfig-provider.ts, which extends the parsed TypeScript options with anexclude: string[]field extracted from the raw config. ThetsOptionstype inswc-compiler.tsandbuild.action.tsis updated to use this new type.swcDefaultsFactoryinswc-defaults.tsnow populatescliOptions.ignorefromtsOptions.exclude, passing the tsconfig excludes to the SWC CLI's--ignoreoption.- New test files added for both
TsConfigProviderandswcDefaultsFactory.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/compiler/helpers/tsconfig-provider.ts |
Adds TsConfigProviderOutput type and parseExclude helper; getByConfigFilename now extracts and validates exclude from raw tsconfig |
lib/compiler/defaults/swc-defaults.ts |
Uses tsOptions.exclude to set SWC CLI's ignore option |
lib/compiler/swc/swc-compiler.ts |
Updates SwcCompilerExtras.tsOptions type from ts.CompilerOptions to TsConfigProviderOutput['options'] |
actions/build.action.ts |
Updates runSwc parameter type; adds (duplicate) TsConfigProviderOutput import |
test/lib/compiler/helpers/tsconfig-provider.spec.ts |
New tests for TsConfigProvider.getByConfigFilename, including exclude parsing logic |
test/lib/compiler/defaults/swc-defaults.spec.ts |
New tests for swcDefaultsFactory, including the new ignore field behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { isModuleAvailable } from '../lib/utils/is-module-available'; | ||
| import { AbstractAction } from './abstract.action'; | ||
| import webpack = require('webpack'); | ||
| import { TsConfigProviderOutput } from '../lib/compiler/helpers/tsconfig-provider'; |
There was a problem hiding this comment.
The TsConfigProviderOutput type is imported from '../lib/compiler/helpers/tsconfig-provider' at line 27, but TsConfigProvider is already imported from the same module at line 10. These two imports from the same module should be combined into a single import statement. Additionally, the new import is placed after the webpack require statement, breaking the grouping of ES module imports. It should be merged with the existing import at line 10.
| expect(result.options.exclude).toEqual(['node_modules', 'dist']); | ||
| expect(result.fileNames).toBeDefined(); | ||
|
|
||
| expect(fs.readFileSync).toHaveBeenCalledWith(configPath); |
There was a problem hiding this comment.
The assertion expect(fs.readFileSync).toHaveBeenCalledWith(configPath) is fragile because TypeScript's internal sys.readFile calls fs.readFileSync with additional arguments (e.g., an encoding parameter like 'utf8'). The toHaveBeenCalledWith matcher requires an exact argument match, so this assertion will fail when TypeScript passes the encoding argument. Consider using expect(fs.readFileSync).toHaveBeenCalledWith(configPath, expect.anything()) or toHaveBeenCalledWith(expect.stringContaining('tsconfig.json'), expect.anything()), or replace the assertion with toHaveBeenCalled() since the actual content verification is done through the returned result.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
When switching a fresh NestJS starter over to use SWC (using the official guide), the
excludefield intsconfig.build.jsonis no longer considered for build output. This means that a file likesrc/app.controller.spec.ts(included in the starter) ends up in the build asdist/app.controller.spec.js, even thoughtsconfig.build.jsoncontains an exclusion glob pattern of"**/*spec.ts".It is expected that switching the NestJS builder from
tsctoswcdoes not cause test files to be included in thenest buildoutput.Issue Number: #2976
What is the new behavior?
The
nest buildcommand uses the parsed TypeScript config'sexcludevalue to generate a default value for theignore@swc/cli option.Does this PR introduce a breaking change?
Unfortunately, it is possible that someone has come to rely on the unexpected behaviour of
tsconfig.build.jsonexcludes not working for SWC compilation. However, I do think it's extremely unlikely. I welcome opposing thoughts.Other information
Full disclosure that I did use an AI agent to generate the tests specifically. I can drop the relevant commits (25f9438, f1107f1) if necessary, but then this feature will not have any tests. I believe we should at leave in a test of the retrieval of
excludefrom atsconfig.json, because the TypeScript API could change in the future – theParsedCommandLine['raw']value we use is currently typed asanyby TypeScript.Also, there is already a similar PR, but it uses potentially more fragile mechanisms to retrieve the
excludevalue from atsconfig.json: