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.

Sign up for GitHub

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

Jump to bottom

fix(config): don't set include property value of tsconfig to empty array #1606

Merged
merged 1 commit into from
May 7, 2020
Merged

fix(config): don't set include property value of tsconfig to empty array #1606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ For example:
{
// ...other configs
"files": [
"my-custom-typings.d.ts".
"my-custom-typings.d.ts",
"my-global-module.ts"
]
}
Expand Down
28 changes: 28 additions & 0 deletions docs/user/config/isolatedModules.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,31 @@ module.exports = {
```

</div></div>

# Performance

Using `isolatedModules: false` comes with a cost of performance comparing to `isolatedModules: true`. There is a way
to improve the performance when using this mode by changing the value of `include` in `tsconfig` which is used by `ts-jest`.
The least amount of files which are provided in `include`, the more performance the test run can gain.

### Example

```json5
// tsconfig.json
{
// ...other configs
include: [
"my-typings/*",
"my-global-modules/*",
]
}
```

## Caveats
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure if writing like this is fine. Would you please check ?

Copy link
Owner

Choose a reason for hiding this comment

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

This looks fine. It's basically your opinion on how this could be helpful under specific conditions. Nothing wrong with it


Limiting the amount of files loaded via `include` can greatly boost performance when running tests. However, the trade off
is `ts-jest` might not recognize all files which are intended to use with `jest`. One can run into issues with custom typings,
global modules, etc...

The suggested solution is what is needed for the test environment should be captured by
glob patterns in `include`, to gain both performance boost and avoid breaking behaviors.
5 changes: 1 addition & 4 deletions e2e/__external-repos__/custom-typings/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,5 @@
"target": "es5",
"module": "commonjs",
"esModuleInterop": true
},
"files": [
"jquery.d.ts"
]
}
}
45 changes: 3 additions & 42 deletions src/config/config-set.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,8 +468,9 @@ describe('typescript', () => {
createConfigSet({ tsJestConfig: tsJest, parentConfig }).parsedTsConfig

it('should read file list from default tsconfig', () => {
// since the default is to lookup for tsconfig, but we set include to [] so we should not have this file in the list
expect(get().fileNames).toEqual([])
// since the default is to lookup for tsconfig,
// we should have this file in the list
expect(get().fileNames).toContain(normalizeSlashes(__filename))
})

it.each(['tsConfig', 'tsconfig'])('should include compiler config from `%s` option key', (key: string) => {
Expand Down Expand Up @@ -610,11 +611,6 @@ describe('readTsConfig', () => {
const conf = cs.readTsConfig()
expect(conf.options.configFilePath).toBeUndefined()
expect(readConfig).not.toHaveBeenCalled()
expect(parseConfig.mock.calls[0][0]).toEqual(
expect.objectContaining({
include: [],
}),
)
expect(parseConfig.mock.calls[0][2]).toBe('/root')
expect(parseConfig.mock.calls[0][4]).toBeUndefined()
})
Expand Down Expand Up @@ -661,11 +657,6 @@ describe('readTsConfig', () => {
expect(conf.options.path).toBe('/root/tsconfig.json')
expect(findConfig.mock.calls[0][0]).toBe('/root')
expect(readConfig.mock.calls[0][0]).toBe('/root/tsconfig.json')
expect(parseConfig.mock.calls[0][0]).toEqual(
expect.objectContaining({
include: [],
}),
)
expect(parseConfig.mock.calls[0][2]).toBe('/root')
expect(parseConfig.mock.calls[0][4]).toBe('/root/tsconfig.json')
expect(conf.options.allowSyntheticDefaultImports).toEqual(true)
Expand All @@ -677,11 +668,6 @@ describe('readTsConfig', () => {
expect(conf.options.path).toBe('/foo/tsconfig.bar.json')
expect(findConfig).not.toBeCalled()
expect(readConfig.mock.calls[0][0]).toBe('/foo/tsconfig.bar.json')
expect(parseConfig.mock.calls[0][0]).toEqual(
expect.objectContaining({
include: [],
}),
)
expect(parseConfig.mock.calls[0][2]).toBe('/foo')
expect(parseConfig.mock.calls[0][4]).toBe('/foo/tsconfig.bar.json')
expect(conf.errors).toMatchSnapshot()
Expand Down Expand Up @@ -710,11 +696,6 @@ describe('readTsConfig', () => {
expect(conf.options.path).toBe('/root/tsconfig.json')
expect(findConfig.mock.calls[0][0]).toBe('/root')
expect(readConfig.mock.calls[0][0]).toBe('/root/tsconfig.json')
expect(parseConfig.mock.calls[0][0]).toEqual(
expect.objectContaining({
include: [],
}),
)
expect(parseConfig.mock.calls[0][2]).toBe('/root')
expect(parseConfig.mock.calls[0][4]).toBe('/root/tsconfig.json')
expect(conf.options.allowSyntheticDefaultImports).toEqual(true)
Expand Down Expand Up @@ -754,11 +735,6 @@ describe('readTsConfig', () => {
expect(conf.options.path).toBe('/root/tsconfig.json')
expect(findConfig.mock.calls[0][0]).toBe('/root')
expect(readConfig.mock.calls[0][0]).toBe('/root/tsconfig.json')
expect(parseConfig.mock.calls[0][0]).toEqual(
expect.objectContaining({
include: [],
}),
)
expect(parseConfig.mock.calls[0][2]).toBe('/root')
expect(parseConfig.mock.calls[0][4]).toBe('/root/tsconfig.json')
expect(conf.options.allowSyntheticDefaultImports).toBeUndefined()
Expand All @@ -770,11 +746,6 @@ describe('readTsConfig', () => {
expect(conf.options.path).toBe('/foo/tsconfig.bar.json')
expect(findConfig).not.toBeCalled()
expect(readConfig.mock.calls[0][0]).toBe('/foo/tsconfig.bar.json')
expect(parseConfig.mock.calls[0][0]).toEqual(
expect.objectContaining({
include: [],
}),
)
expect(parseConfig.mock.calls[0][2]).toBe('/foo')
expect(parseConfig.mock.calls[0][4]).toBe('/foo/tsconfig.bar.json')
expect(conf.errors).toEqual([])
Expand Down Expand Up @@ -803,11 +774,6 @@ describe('readTsConfig', () => {
expect(conf.options.path).toBe('/root/tsconfig.json')
expect(findConfig.mock.calls[0][0]).toBe('/root')
expect(readConfig.mock.calls[0][0]).toBe('/root/tsconfig.json')
expect(parseConfig.mock.calls[0][0]).toEqual(
expect.objectContaining({
include: [],
}),
)
expect(parseConfig.mock.calls[0][2]).toBe('/root')
expect(parseConfig.mock.calls[0][4]).toBe('/root/tsconfig.json')
expect(conf.errors).toEqual([])
Expand All @@ -819,11 +785,6 @@ describe('readTsConfig', () => {
expect(conf.options.path).toBe('/foo/tsconfig.bar.json')
expect(findConfig).not.toBeCalled()
expect(readConfig.mock.calls[0][0]).toBe('/foo/tsconfig.bar.json')
expect(parseConfig.mock.calls[0][0]).toEqual(
expect.objectContaining({
include: [],
}),
)
expect(parseConfig.mock.calls[0][2]).toBe('/foo')
expect(parseConfig.mock.calls[0][4]).toBe('/foo/tsconfig.bar.json')
expect(conf.errors).toEqual([])
Expand Down
7 changes: 1 addition & 6 deletions src/config/config-set.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ export class ConfigSet {
resolvedConfigFile?: string | null,
noProject?: boolean | null,
): ParsedCommandLine {
let config = { compilerOptions: {}, include: [] }
let config = { compilerOptions: {} }
let basePath = normalizeSlashes(this.rootDir)
let configFileName: string | undefined
const ts = this.compilerModule
Expand Down Expand Up @@ -741,11 +741,6 @@ export class ConfigSet {
...config.compilerOptions,
...compilerOptions,
}
/**
* Always set include to empty array so fileNames after parseJsonConfigFileContent only contains the least minimum initial
* files to utilize LanguageService incremental feature
*/
config.include = []

// parse json, merge config extending others, ...
const result = ts.parseJsonConfigFileContent(config, ts.sys, basePath, undefined, configFileName)
Expand Down