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.

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

Explicitly avoid canonicalizing paths during configuration handling #18316

Merged
merged 2 commits into from
Sep 7, 2017

Conversation

weswigham
Copy link
Member

According to this comment, forceConsistentCasingInFileNames relies on paths not being canonicalized until later in the process; so this replaces new usages of getCanonicalFileName in the configuration parser with an identity function, so as to avoid canonicalizing the path when it is not needed.

@weswigham weswigham requested a review from a user September 7, 2017 18:32
@weswigham
Copy link
Member Author

@Andy-MS Would you take a look at this?

@@ -1228,6 +1228,9 @@ namespace ts {
/** Does nothing. */
export function noop(): void {}

/** Passes thru argument. */
export function identity<T>(x: T) { return x; }
Copy link

Choose a reason for hiding this comment

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

This could also replace identityMapper in checker.ts.

@@ -1577,7 +1577,7 @@ namespace ts {
errors.push(createCompilerDiagnostic(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, "extends", "string"));
}
else {
const newBase = configFileName ? getDirectoryPath(toPath(configFileName, basePath, getCanonicalFileName)) : basePath;
const newBase = configFileName ? getDirectoryPath(toPath(configFileName, basePath, identity)) : basePath;
Copy link

Choose a reason for hiding this comment

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

Might want to create a local toPath function delegating to ts.toPath that explains why we use identity here.

@@ -1228,6 +1228,9 @@ namespace ts {
/** Does nothing. */
export function noop(): void {}

/** Passes thru argument. */
Copy link

Choose a reason for hiding this comment

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

through; or just Returns its argument.

@weswigham weswigham merged commit 27f9cdb into microsoft:master Sep 7, 2017
@weswigham weswigham deleted the do-not-canonicalize-paths branch September 7, 2017 22:54
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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.

2 participants