CR feedback

zhengbli · zhengbli · commit 5981d8e60c6c · 2016-02-26T14:27:37.000-08:00
diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts
@@ -616,10 +616,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));
@@ -668,7 +668,7 @@ namespace ts {
                                 break;
                             case "object":
                                 // "object" options with 'isFilePath' = true expected to be string arrays
-                                value = ConvertJsonOptionToStringArray(opt.name, value, errors, (element) => normalizePath(combinePaths(basePath, element)));
+                                value = convertJsonOptionToStringArray(opt.name, value, errors, (element) => normalizePath(combinePaths(basePath, element)));
                                 break;
                         }
                         if (value === "") {
@@ -689,7 +689,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
`@@ -616,10 +616,10 @@ namespace ts {`
`616`	`616`	`}`
`617`	`617`	`}`
`618`	`618`	`else if (id === "include") {`
`619`		`- options.include = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
	`619`	`+ options.include = convertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
`620`	`620`	`}`
`621`	`621`	`else if (id === "exclude") {`
`622`		`- options.exclude = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
	`622`	`+ options.exclude = convertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
`623`	`623`	`}`
`624`	`624`	`else {`
`625`	`625`	`errors.push(createCompilerDiagnostic(Diagnostics.Unknown_typing_option_0, id));`
`@@ -668,7 +668,7 @@ namespace ts {`
`668`	`668`	`break;`
`669`	`669`	`case "object":`
`670`	`670`	`// "object" options with 'isFilePath' = true expected to be string arrays`
`671`		`- value = ConvertJsonOptionToStringArray(opt.name, value, errors, (element) => normalizePath(combinePaths(basePath, element)));`
	`671`	`+ value = convertJsonOptionToStringArray(opt.name, value, errors, (element) => normalizePath(combinePaths(basePath, element)));`
`672`	`672`	`break;`
`673`	`673`	`}`
`674`	`674`	`if (value === "") {`
`@@ -689,7 +689,7 @@ namespace ts {`
`689`	`689`	`return { options, errors };`
`690`	`690`	`}`
`691`	`691`
`692`		`- function ConvertJsonOptionToStringArray(optionName: string, optionJson: any, errors: Diagnostic[], func?: (element: string) => string): string[] {`
	`692`	`+ function convertJsonOptionToStringArray(optionName: string, optionJson: any, errors: Diagnostic[], func?: (element: string) => string): string[] {`
`693`	`693`	`const items: string[] = [];`
`694`	`694`	`let invalidOptionType = false;`
`695`	`695`	`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`	`}`