Skip to content

Clean up uses of ensureScriptKind #16714

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

Merged
2 commits merged into from
Jun 28, 2017
Merged

Clean up uses of ensureScriptKind #16714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bbdeb4
Clean up uses of ensureScriptKind
Jun 23, 2017
3b58c02
Remove unneeded parentheses
Jun 28, 2017
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
4 changes: 2 additions & 2 deletions src/compiler/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2118,14 +2118,14 @@ namespace ts {
return absolute.substring(0, absolute.lastIndexOf(directorySeparator, wildcardOffset));
}

export function ensureScriptKind(fileName: string, scriptKind?: ScriptKind): ScriptKind {
export function ensureScriptKind(fileName: string, scriptKind: ScriptKind | undefined): ScriptKind {
// Using scriptKind as a condition handles both:
// - 'scriptKind' is unspecified and thus it is `undefined`
// - 'scriptKind' is set and it is `Unknown` (0)
// If the 'scriptKind' is 'undefined' or 'Unknown' then we attempt
// to get the ScriptKind from the file name. If it cannot be resolved
// from the file name then the default 'TS' script kind is returned.
return (scriptKind || getScriptKindFromFileName(fileName)) || ScriptKind.TS;
return scriptKind || getScriptKindFromFileName(fileName) || ScriptKind.TS;
}

export function getScriptKindFromFileName(fileName: string): ScriptKind {
Expand Down
5 changes: 2 additions & 3 deletions src/services/jsTyping.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ namespace ts.JsTyping {
// Only infer typings for .js and .jsx files
fileNames = mapDefined(fileNames, fileName => {
const path = normalizePath(fileName);
const kind = ensureScriptKind(path, getScriptKindFromFileName(path));
if (kind === ScriptKind.JS || kind === ScriptKind.JSX) {
if (hasJavaScriptFileExtension(path)) {
return path;
}
});
Expand Down Expand Up @@ -191,7 +190,7 @@ namespace ts.JsTyping {
}
}

const hasJsxFile = forEach(fileNames, f => ensureScriptKind(f, getScriptKindFromFileName(f)) === ScriptKind.JSX);
const hasJsxFile = some(fileNames, f => fileExtensionIs(f, Extension.Jsx));
if (hasJsxFile) {
addInferredTyping("react");
}
Expand Down
9 changes: 1 addition & 8 deletions src/services/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1303,14 +1303,7 @@ namespace ts {
export function getScriptKind(fileName: string, host?: LanguageServiceHost): ScriptKind {
// First check to see if the script kind was specified by the host. Chances are the host
// may override the default script kind for the file extension.
let scriptKind: ScriptKind;
if (host && host.getScriptKind) {
scriptKind = host.getScriptKind(fileName);
}
if (!scriptKind) {
scriptKind = getScriptKindFromFileName(fileName);
Copy link
Member

Choose a reason for hiding this comment

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

is host.getScriptKind the same as getScriptKindFromFileName here? If not, where is host.getScriptKind defined? (I'm trying to figure out why you removed the fallback to getScriptKindFromFileName.)

Copy link
Author

@ghost ghost Jun 28, 2017 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

There's some complex logic here, but I think what I did is equivalent. In both cases:

  • If host.getScriptKind(fileName) is defined, uses that.
  • Else, uses getScriptKindFromFileName(fileName).
  • If that fails, uses ScriptKind.TS.

ensureScriptKind itself handles falling back to getScriptKindFromFileName, so there's no need to do that here.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, point 3 is what I missed. Thanks.

}
return ensureScriptKind(fileName, scriptKind);
return ensureScriptKind(fileName, host && host.getScriptKind && host.getScriptKind(fileName));
}

export function getFirstNonSpaceCharacterPosition(text: string, position: number) {
Expand Down