CR feedback

jramsay · jramsay · commit a7a13296e6b1 · 2016-03-08T17:40:03.000-08:00
diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts
@@ -590,10 +590,10 @@ namespace ts {
                         }
                     }
                     else if (id === "include") {
-                        options.include = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);
+                        options.include = convertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);
                     }
                     else if (id === "exclude") {
-                        options.exclude = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);
+                        options.exclude = convertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);
                     }
                     else {
                         errors.push(createCompilerDiagnostic(Diagnostics.Unknown_typing_option_0, id));
@@ -636,9 +636,9 @@ namespace ts {
                         }
                     }
                     if (opt.isFilePath) {
-                       value = normalizePath(combinePaths(basePath, value));
+                        value = normalizePath(combinePaths(basePath, value));
                        if (value === "") {
-                            value = ".";
+                           value = ".";
                         }
                     }
                     options[opt.name] = value;
@@ -655,7 +655,7 @@ namespace ts {
         return { options, errors };
     }
 
-    function ConvertJsonOptionToStringArray(optionName: string, optionJson: any, errors: Diagnostic[], func?: (element: string) => string): string[] {
+    function convertJsonOptionToStringArray(optionName: string, optionJson: any, errors: Diagnostic[], func?: (element: string) => string): string[] {
         const items: string[] = [];
         let invalidOptionType = false;
         if (!isArray(optionJson)) {
diff --git a/src/services/jsTyping.ts b/src/services/jsTyping.ts
@@ -193,8 +193,8 @@ namespace ts.JsTyping {
                 mergeTypings(filter(cleanedTypingNames, f => hasProperty(safeList, f)));
             }
 
-            const jsxFileNames = filter(fileNames, f => scriptKindIs(f, /*LanguageServiceHost*/ undefined, ScriptKind.JSX));
-            if (jsxFileNames.length > 0) {
+            const hasJsxFile = forEach(fileNames, f => scriptKindIs(f, /*LanguageServiceHost*/ undefined, ScriptKind.JSX));
+            if (hasJsxFile) {
                 mergeTypings(["react"]);
             }
         }
@@ -214,6 +214,7 @@ namespace ts.JsTyping {
                 filter(
                     host.readDirectory(nodeModulesPath, /*extension*/ undefined, /*exclude*/ undefined, /*depth*/ 2),
                     f => ts.getBaseFileName(f) === "package.json");
+
             for (const packageJsonFile of packageJsonFiles) {
                 const packageJsonDict = tryParseJson(packageJsonFile, host);
                 if (!packageJsonDict) { continue; }
@@ -247,13 +248,6 @@ namespace ts.JsTyping {
             if (!options) {
                 return;
             }
-
-            if (options.jsx === JsxEmit.React) {
-                typingNames.push("react");
-            }
-            if (options.moduleResolution === ModuleResolutionKind.NodeJs) {
-                typingNames.push("node");
-            }
             mergeTypings(typingNames);
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -590,10 +590,10 @@ namespace ts {`
`590`	`590`	`}`
`591`	`591`	`}`
`592`	`592`	`else if (id === "include") {`
`593`		`- options.include = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
	`593`	`+ options.include = convertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
`594`	`594`	`}`
`595`	`595`	`else if (id === "exclude") {`
`596`		`- options.exclude = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
	`596`	`+ options.exclude = convertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
`597`	`597`	`}`
`598`	`598`	`else {`
`599`	`599`	`errors.push(createCompilerDiagnostic(Diagnostics.Unknown_typing_option_0, id));`
`@@ -636,9 +636,9 @@ namespace ts {`
`636`	`636`	`}`
`637`	`637`	`}`
`638`	`638`	`if (opt.isFilePath) {`
`639`		`- value = normalizePath(combinePaths(basePath, value));`
	`639`	`+ value = normalizePath(combinePaths(basePath, value));`
`640`	`640`	`if (value === "") {`
`641`		`- value = ".";`
	`641`	`+ value = ".";`
`642`	`642`	`}`
`643`	`643`	`}`
`644`	`644`	`options[opt.name] = value;`
`@@ -655,7 +655,7 @@ namespace ts {`
`655`	`655`	`return { options, errors };`
`656`	`656`	`}`
`657`	`657`
`658`		`- function ConvertJsonOptionToStringArray(optionName: string, optionJson: any, errors: Diagnostic[], func?: (element: string) => string): string[] {`
	`658`	`+ function convertJsonOptionToStringArray(optionName: string, optionJson: any, errors: Diagnostic[], func?: (element: string) => string): string[] {`
`659`	`659`	`const items: string[] = [];`
`660`	`660`	`let invalidOptionType = false;`
`661`	`661`	`if (!isArray(optionJson)) {`
Original file line number	Diff line number	Diff line change
`@@ -193,8 +193,8 @@ namespace ts.JsTyping {`
`193`	`193`	`mergeTypings(filter(cleanedTypingNames, f => hasProperty(safeList, f)));`
`194`	`194`	`}`
`195`	`195`
`196`		`- const jsxFileNames = filter(fileNames, f => scriptKindIs(f, /LanguageServiceHost/ undefined, ScriptKind.JSX));`
`197`		`- if (jsxFileNames.length > 0) {`
	`196`	`+ const hasJsxFile = forEach(fileNames, f => scriptKindIs(f, /LanguageServiceHost/ undefined, ScriptKind.JSX));`
	`197`	`+ if (hasJsxFile) {`
`198`	`198`	`mergeTypings(["react"]);`
`199`	`199`	`}`
`200`	`200`	`}`
`@@ -214,6 +214,7 @@ namespace ts.JsTyping {`
`214`	`214`	`filter(`
`215`	`215`	`host.readDirectory(nodeModulesPath, /extension/ undefined, /exclude/ undefined, /depth/ 2),`
`216`	`216`	`f => ts.getBaseFileName(f) === "package.json");`
	`217`	`+`
`217`	`218`	`for (const packageJsonFile of packageJsonFiles) {`
`218`	`219`	`const packageJsonDict = tryParseJson(packageJsonFile, host);`
`219`	`220`	`if (!packageJsonDict) { continue; }`
`@@ -247,13 +248,6 @@ namespace ts.JsTyping {`
`247`	`248`	`if (!options) {`
`248`	`249`	`return;`
`249`	`250`	`}`
`250`		`-`
`251`		`- if (options.jsx === JsxEmit.React) {`
`252`		`- typingNames.push("react");`
`253`		`- }`
`254`		`- if (options.moduleResolution === ModuleResolutionKind.NodeJs) {`
`255`		`- typingNames.push("node");`
`256`		`- }`
`257`	`251`	`mergeTypings(typingNames);`
`258`	`252`	`}`
`259`	`253`	`}`