Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Tsconfig inheritance: Do not resolve included files in an inherited tsconfig#15139

Merged
4 commits merged into
mastermicrosoft/TypeScript:masterfrom
config-extensionmicrosoft/TypeScript:config-extensionCopy head branch name to clipboard
May 8, 2017
Merged

Tsconfig inheritance: Do not resolve included files in an inherited tsconfig#15139
4 commits merged into
mastermicrosoft/TypeScript:masterfrom
config-extensionmicrosoft/TypeScript:config-extensionCopy head branch name to clipboard

Conversation

@ghost

@ghost ghost commented Apr 11, 2017

Copy link
Copy Markdown

Fixes #14539

The problem was that parseJsonConfigFileContent recursively calls itself.
This function doesn't just parse the JSON config, it also resolves the files of everything in includes. Since we don't use the resolved includes from a parent config, this is wasteful.
Factored the parts out that just parse the config, so that when we parse an extended config, we don't try resolving includes for that.

You can verify the fix with:

git clone https://github.com/brandtotal/missbehave-tsconfig-extend.git
cd TypeScript; co config-extension
gulp clean; gulp local
# Good
strace -e stat node built/local/tsc.js -p ../missbehave-tsconfig-extend/bad-test-case/proj1/tsconfig.json
# Bad: see stat of `this-file-is-not-expected-to-be-lstat-by-tsc.ts`
strace -e stat tsc -p ../missbehave-tsconfig-extend/bad-test-case/proj1/tsconfig.json

I also noticed that we are modifying the input to this function; stopping that would technically be a breaking change, but it doesn't seem to have been documented in the first place, so probably a good idea to do that in another PR?

@ghost ghost assigned zhengbli Apr 12, 2017
Comment thread src/compiler/commandLineParser.ts Outdated
let options: CompilerOptions = convertCompilerOptionsFromJsonWorker(json["compilerOptions"], basePath, errors, configFileName);
let options = (() => {
const { include, exclude, files, options } = parseConfig(json, host, basePath, configFileName, resolutionStack, errors);
if (include) json.include = include;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

braces

Comment thread src/compiler/commandLineParser.ts Outdated
host: ParseConfigHost,
basePath: string,
configFileName: string,
resolutionStack: Path[] = [],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Put the parameter with default values last? Otherwise the default value will never be used as it can not be emitted.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Why is that not a compile error?

Comment thread src/compiler/commandLineParser.ts Outdated
}

// If the path isn't a rooted or relative path, don't try to resolve it (we reserve the right to special case module-id like paths in the future)
if (!(isRootedDiskPath(extended) || startsWith(normalizeSlashes(extended), "./") || startsWith(normalizeSlashes(extended), "../"))) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

don't normalize twice

@ghost ghost assigned sheetalkamat and unassigned zhengbli May 8, 2017
@ghost ghost merged commit a4b9110 into master May 8, 2017
@ghost ghost deleted the config-extension branch May 8, 2017 18:32
@microsoft microsoft locked and limited conversation to collaborators Jun 21, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tsconfig extends property makes tsc scanning files its shouldn't (causing serious slowdown on compilation)

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.