Merge pull request #4445 from NativeScript/fatme/merge-release

chore: merge release into master

Author: Fatme

lib/common/http-client.ts

@@ -56,30 +56,30 @@ export class HttpClient implements Server.IHttpClient {
 			};
 		}
 
-		const unmodifiedOptions = _.clone(options);
+		const clonedOptions = _.cloneDeep(options);
 
-		if (options.url) {
-			const urlParts = url.parse(options.url);
+		if (clonedOptions.url) {
+			const urlParts = url.parse(clonedOptions.url);
 			if (urlParts.protocol) {
-				options.proto = urlParts.protocol.slice(0, -1);
+				clonedOptions.proto = urlParts.protocol.slice(0, -1);
 			}
-			options.host = urlParts.hostname;
-			options.port = urlParts.port;
-			options.path = urlParts.path;
+			clonedOptions.host = urlParts.hostname;
+			clonedOptions.port = urlParts.port;
+			clonedOptions.path = urlParts.path;
 		}
 
-		const requestProto = options.proto || "http";
-		const body = options.body;
-		delete options.body;
-		let pipeTo = options.pipeTo;
-		delete options.pipeTo;
+		const requestProto = clonedOptions.proto || "http";
+		const body = clonedOptions.body;
+		delete clonedOptions.body;
+		let pipeTo = options.pipeTo; // Use the real stream because the _.cloneDeep can't clone the internal state of a stream.
+		delete clonedOptions.pipeTo;
 
 		const cliProxySettings = await this.$proxyService.getCache();
 
-		options.headers = options.headers || {};
-		const headers = options.headers;
+		clonedOptions.headers = clonedOptions.headers || {};
+		const headers = clonedOptions.headers;
 
-		await this.useProxySettings(proxySettings, cliProxySettings, options, headers, requestProto);
+		await this.useProxySettings(proxySettings, cliProxySettings, clonedOptions, headers, requestProto);
 
 		if (!headers.Accept || headers.Accept.indexOf("application/json") < 0) {
 			if (headers.Accept) {
@@ -115,21 +115,21 @@ export class HttpClient implements Server.IHttpClient {
 				isResolved: () => false
 			};
 
-			if (options.timeout) {
+			clonedOptions.url = clonedOptions.url || `${clonedOptions.proto}://${clonedOptions.host}${clonedOptions.path}`;
+			if (clonedOptions.timeout) {
 				timerId = setTimeout(() => {
-					this.setResponseResult(promiseActions, cleanupRequestData, { err: new Error(`Request to ${unmodifiedOptions.url} timed out.`) });
-				}, options.timeout);
+					this.setResponseResult(promiseActions, cleanupRequestData, { err: new Error(`Request to ${clonedOptions.url} timed out.`) });
+				}, clonedOptions.timeout);
 				cleanupRequestData.timers.push(timerId);
 
-				delete options.timeout;
+				delete clonedOptions.timeout;
 			}
 
-			options.url = options.url || `${options.proto}://${options.host}${options.path}`;
-			options.encoding = null;
-			options.followAllRedirects = true;
+			clonedOptions.encoding = null;
+			clonedOptions.followAllRedirects = true;
 
-			this.$logger.trace("httpRequest: %s", util.inspect(options));
-			const requestObj = request(options);
+			this.$logger.trace("httpRequest: %s", util.inspect(clonedOptions));
+			const requestObj = request(clonedOptions);
 			cleanupRequestData.req = requestObj;
 
 			requestObj
@@ -151,7 +151,7 @@ export class HttpClient implements Server.IHttpClient {
 
 					stuckRequestTimerId = setTimeout(() => {
 						this.setResponseResult(promiseActions, cleanupRequestData, { err: new Error(HttpClient.STUCK_REQUEST_ERROR_MESSAGE) });
-					}, options.timeout || HttpClient.STUCK_REQUEST_TIMEOUT);
+					}, clonedOptions.timeout || HttpClient.STUCK_REQUEST_TIMEOUT);
 
 					cleanupRequestData.timers.push(stuckRequestTimerId);
 
@@ -230,6 +230,7 @@ export class HttpClient implements Server.IHttpClient {
 		const response = await result;
 
 		if (helpers.isResponseRedirect(response.response)) {
+			const unmodifiedOptions = _.cloneDeep(options);
 			if (response.response.statusCode === HttpStatusCodes.SEE_OTHER) {
 				unmodifiedOptions.method = "GET";
 			}

lib/services/doctor-service.ts

@@ -141,8 +141,10 @@ export class DoctorService implements IDoctorService {
 		for (const file of files) {
 			const fileContent = this.$fs.readText(file);
 			const strippedComments = helpers.stripComments(fileContent);
-			const linesWithRequireStatements = strippedComments
+			const linesToCheck = _.flatten(strippedComments
 				.split(/\r?\n/)
+				.map(line => line.split(";")));
+			const linesWithRequireStatements = linesToCheck
 				.filter(line => /\btns-core-modules\b/.exec(line) === null && (/\bimport\b/.exec(line) || /\brequire\b/.exec(line)));
 
 			for (const line of linesWithRequireStatements) {

lib/services/project-data-service.ts

@@ -1,7 +1,15 @@
 import * as path from "path";
 import { ProjectData } from "../project-data";
 import { exported } from "../common/decorators";
-import { NATIVESCRIPT_PROPS_INTERNAL_DELIMITER, AssetConstants, SRC_DIR, RESOURCES_DIR, MAIN_DIR, CLI_RESOURCES_DIR_NAME, ProjectTypes } from "../constants";
+import {
+	NATIVESCRIPT_PROPS_INTERNAL_DELIMITER,
+	AssetConstants, SRC_DIR,
+	RESOURCES_DIR,
+	MAIN_DIR,
+	CLI_RESOURCES_DIR_NAME,
+	ProjectTypes,
+	NODE_MODULES_FOLDER_NAME
+} from "../constants";
 
 interface IProjectFileData {
 	projectData: any;
@@ -124,6 +132,7 @@ export class ProjectDataService implements IProjectDataService {
 			supportedFileExtension = ".ts";
 		}
 
+		const pathToProjectNodeModules = path.join(projectDir, NODE_MODULES_FOLDER_NAME);
 		const files = this.$fs.enumerateFilesInDirectorySync(
 			projectData.appDirectoryPath,
 			(filePath, fstat) => {
@@ -132,6 +141,12 @@ export class ProjectDataService implements IProjectDataService {
 				}
 
 				if (fstat.isDirectory()) {
+					if (filePath === pathToProjectNodeModules) {
+						// we do not want to get the files from node_modules directory of the project.
+						// We'll get here only when you have nsconfig.json with appDirectoryPath set to "."
+						return false;
+					}
+
 					return true;
 				}
 

test/services/doctor-service.ts

@@ -49,7 +49,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: 'const application = require("application");'
 				},
-				expectedShortImports: [{ file: "file1", line: 'const application = require("application");' }]
+				expectedShortImports: [{ file: "file1", line: 'const application = require("application")' }]
 			},
 			{
 				filesContents: {
@@ -61,7 +61,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: 'const Observable = require("data/observable").Observable;'
 				},
-				expectedShortImports: [{ file: "file1", line: 'const Observable = require("data/observable").Observable;' }]
+				expectedShortImports: [{ file: "file1", line: 'const Observable = require("data/observable").Observable' }]
 			},
 			{
 				filesContents: {
@@ -73,7 +73,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: 'import * as application from "application";'
 				},
-				expectedShortImports: [{ file: "file1", line: 'import * as application from "application";' }]
+				expectedShortImports: [{ file: "file1", line: 'import * as application from "application"' }]
 			},
 			{
 				filesContents: {
@@ -85,7 +85,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: 'import { run } from "application";'
 				},
-				expectedShortImports: [{ file: "file1", line: 'import { run } from "application";' }]
+				expectedShortImports: [{ file: "file1", line: 'import { run } from "application"' }]
 			},
 			{
 				filesContents: {
@@ -98,7 +98,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: "import { run } from 'application';"
 				},
-				expectedShortImports: [{ file: "file1", line: "import { run } from 'application';" }]
+				expectedShortImports: [{ file: "file1", line: "import { run } from 'application'" }]
 			},
 			{
 				// Using single quotes
@@ -114,8 +114,8 @@ const Observable = require("data/observable").Observable;
 `
 				},
 				expectedShortImports: [
-					{ file: "file1", line: 'const application = require("application");' },
-					{ file: "file1", line: 'const Observable = require("data/observable").Observable;' },
+					{ file: "file1", line: 'const application = require("application")' },
+					{ file: "file1", line: 'const Observable = require("data/observable").Observable' },
 				]
 			},
 			{
@@ -125,7 +125,7 @@ const Observable = require("tns-core-modules/data/observable").Observable;
 `
 				},
 				expectedShortImports: [
-					{ file: "file1", line: 'const application = require("application");' },
+					{ file: "file1", line: 'const application = require("application")' },
 				]
 			},
 			{
@@ -137,8 +137,8 @@ const Observable = require("tns-core-modules/data/observable").Observable;
 const Observable = require("data/observable").Observable;`
 				},
 				expectedShortImports: [
-					{ file: "file1", line: 'const application = require("application");' },
-					{ file: "file2", line: 'const Observable = require("data/observable").Observable;'  },
+					{ file: "file1", line: 'const application = require("application")' },
+					{ file: "file2", line: 'const Observable = require("data/observable").Observable' },
 				]
 			},
 			{
@@ -150,7 +150,45 @@ const Observable = require("tns-core-modules/data/observable").Observable;
 					file2: `const application = require("some-name-tns-core-modules-widgets/application");
 const Observable = require("tns-core-modules-widgets/data/observable").Observable;`
 				},
-				expectedShortImports: [ ]
+				expectedShortImports: []
+			},
+			{
+				filesContents: {
+					// several statements on one line
+					file1: 'const _ = require("lodash");console.log("application");'
+				},
+				expectedShortImports: []
+			},
+			{
+				filesContents: {
+					// several statements on one line with actual short imports
+					file1: 'const _ = require("lodash");const application = require("application");console.log("application");',
+					file2: 'const _ = require("lodash");const application = require("application");const Observable = require("data/observable").Observable;'
+				},
+				expectedShortImports: [
+					{ file: "file1", line: 'const application = require("application")' },
+					{ file: "file2", line: 'const application = require("application")' },
+					{ file: "file2", line: 'const Observable = require("data/observable").Observable' },
+				]
+			},
+			{
+				filesContents: {
+					// several statements on one line withoutshort imports
+					file1: 'const _ = require("lodash");const application = require("tns-core-modules/application");console.log("application");',
+					file2: 'const _ = require("lodash");const application = require("tns-core-modules/application");const Observable = require("tns-core-modules/data/observable").Observable;'
+				},
+				expectedShortImports: []
+			},
+			{
+				// Incorrect behavior, currently by design
+				// In case you have a multiline string and one of the lines matches our RegExp we'll detect it as short import
+				filesContents: {
+					file1: 'const _ = require("lodash");const application = require("application");console.log("application");console.log(`this is line\nyou should import some long words here "application" module and other words here`)',
+				},
+				expectedShortImports: [
+					{ file: "file1", line: 'const application = require("application")' },
+					{ file: "file1", line: 'you should import some long words here "application" module and other words here`)' },
+				]
 			},
 		];