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

getScriptInfoOrConfig: Canonicalize tsconfig path before lookup #26280

Merged
merged 10 commits into from
Jan 9, 2019

Conversation

ghost
Copy link

@ghost ghost commented Aug 7, 2018

May fix the remaining issue in #24857

@ghost ghost requested a review from sheetalkamat August 7, 2018 23:14
@ghost ghost force-pushed the tsconfig_canonicalpath branch from af2dfe9 to a06fcb7 Compare August 7, 2018 23:14
@ghost
Copy link
Author

ghost commented Aug 8, 2018

The error in travis has no stacktrace and doesn't occur when I run the test myself...

@ghost ghost force-pushed the tsconfig_canonicalpath branch from a06fcb7 to 77494d6 Compare August 9, 2018 15:20
@ghost
Copy link
Author

ghost commented Aug 9, 2018

Ran jake runtests (rather than jake runtests --t tsserverProjectSystem to avoid a situation like #10683) on windows and still see no errors locally.

@@ -1804,7 +1804,7 @@ namespace ts.server {
const path = toNormalizedPath(uncheckedFileName);
const info = this.getScriptInfoForNormalizedPath(path);
if (info) return info;
const configProject = this.configuredProjects.get(uncheckedFileName);
const configProject = this.configuredProjects.get(this.toCanonicalFileName(uncheckedFileName));
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is right infact you want to use 'path' here as the key I think.

Copy link
Author

Choose a reason for hiding this comment

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

It looks like I still at least need this.toCanonicalFileName(path). The key comes from project.canonicalConfigPath which is asNormalizedPath(projectService.toCanonicalFileName(configFileName));

@ghost
Copy link
Author

ghost commented Sep 17, 2018

@RyanCavanaugh Any guess what might be going on here? The error is TypeError: Cannot read property 'call' of undefined with no stack trace. Can't reproduce on windows or linux. Tested with jake runtests light=false t=tsserver.

@@ -1884,7 +1884,7 @@ namespace ts.server {
const path = toNormalizedPath(uncheckedFileName);
const info = this.getScriptInfoForNormalizedPath(path);
if (info) return info;
const configProject = this.configuredProjects.get(uncheckedFileName);
const configProject = this.configuredProjects.get(this.toCanonicalFileName(path));
Copy link
Member

Choose a reason for hiding this comment

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

This should this.toPath(uncheckedFileName) instead of just canonicalizing file name, (you need to handle current directory as well though in most cases unchecked file is absolute path but toPath is more correct version)

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.2 milestone Sep 17, 2018
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.

3 participants