Skip to content

Commit 0fabc39

Browse files
Merge pull request #4372 from NativeScript/vladimirov/short-imports-warn
feat: detect and report short imports used in application
2 parents 534b2db + f50f22d commit 0fabc39

15 files changed

+524
-15
lines changed

lib/common/declarations.d.ts

+7
Original file line numberDiff line numberDiff line change
@@ -1168,6 +1168,13 @@ interface IDoctorService {
11681168
* @returns {Promise<boolean>} true if the environment is properly configured for local builds
11691169
*/
11701170
canExecuteLocalBuild(platform?: string, projectDir?: string, runtimeVersion?: string): Promise<boolean>;
1171+
1172+
/**
1173+
* Checks and notifies users for deprecated short imports in their applications.
1174+
* @param {string} projectDir Path to the application.
1175+
* @returns {void}
1176+
*/
1177+
checkForDeprecatedShortImportsInAppDir(projectDir: string): void;
11711178
}
11721179

11731180
interface IUtils {

lib/common/helpers.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ import * as crypto from "crypto";
88
import * as _ from "lodash";
99

1010
const Table = require("cli-table");
11+
const STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
12+
13+
export function stripComments(content: string): string {
14+
const newContent = content.replace(STRIP_COMMENTS, "");
15+
return newContent;
16+
}
1117

1218
export function doesCurrentNpmCommandMatch(patterns?: RegExp[]): boolean {
1319
const currentNpmCommandArgv = getCurrentNpmCommandArgv();
@@ -682,7 +688,6 @@ const CONSTRUCTOR_ARGS = /constructor\s*([^\(]*)\(\s*([^\)]*)\)/m;
682688
const FN_NAME_AND_ARGS = /^(?:function)?\s*([^\(]*)\(\s*([^\)]*)\)\s*(=>)?\s*[{_]/m;
683689
const FN_ARG_SPLIT = /,/;
684690
const FN_ARG = /^\s*(_?)(\S+?)\1\s*$/;
685-
const STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
686691

687692
export function annotate(fn: any) {
688693
let $inject: any,

lib/common/test/unit-tests/helpers.ts

+36
Original file line numberDiff line numberDiff line change
@@ -825,4 +825,40 @@ describe("helpers", () => {
825825
});
826826
});
827827
});
828+
829+
describe("stripComments", () => {
830+
const testData: ITestData[] = [
831+
{
832+
input: `// this is comment,
833+
const test = require("./test");`,
834+
expectedResult: `\nconst test = require("./test");`
835+
},
836+
{
837+
input: `/* this is multiline
838+
comment */
839+
const test = require("./test");`,
840+
expectedResult: `\nconst test = require("./test");`
841+
},
842+
{
843+
input: `/* this is multiline
844+
comment
845+
// with inner one line comment inside it
846+
the multiline comment finishes here
847+
*/
848+
const test = require("./test");`,
849+
expectedResult: `\nconst test = require("./test");`
850+
},
851+
852+
{
853+
input: `const test /*inline comment*/ = require("./test");`,
854+
expectedResult: `const test = require("./test");`
855+
},
856+
];
857+
858+
it("strips comments correctly", () => {
859+
testData.forEach(testCase => {
860+
assertTestData(testCase, helpers.stripComments);
861+
});
862+
});
863+
});
828864
});

lib/constants.ts

+6
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ export const NgFlavorName = "Angular";
116116
export const VueFlavorName = "Vue.js";
117117
export const TsFlavorName = "Plain TypeScript";
118118
export const JsFlavorName = "Plain JavaScript";
119+
export class ProjectTypes {
120+
public static NgFlavorName = NgFlavorName;
121+
public static VueFlavorName = VueFlavorName;
122+
public static TsFlavorName = "Pure TypeScript";
123+
public static JsFlavorName = "Pure JavaScript";
124+
}
119125
export const BUILD_OUTPUT_EVENT_NAME = "buildOutput";
120126
export const CONNECTION_ERROR_EVENT_NAME = "connectionError";
121127
export const USER_INTERACTION_NEEDED_EVENT_NAME = "userInteractionNeeded";

lib/definitions/project.d.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,13 @@ interface IProjectDataService {
172172
* @returns {Promise<IAssetGroup>} An object describing the current asset structure for Android.
173173
*/
174174
getAndroidAssetsStructure(opts: IProjectDir): Promise<IAssetGroup>;
175+
176+
/**
177+
* Returns array with paths to all `.js` or `.ts` files in application's app directory.
178+
* @param {string} projectDir Path to application.
179+
* @returns {string[]} Array of paths to `.js` or `.ts` files.
180+
*/
181+
getAppExecutableFiles(projectDir: string): string[];
175182
}
176183

177184
interface IAssetItem {
@@ -535,9 +542,9 @@ interface ICocoaPodsService {
535542

536543
/**
537544
* Merges pod's xcconfig file into project's xcconfig file
538-
* @param projectData
539-
* @param platformData
540-
* @param opts
545+
* @param projectData
546+
* @param platformData
547+
* @param opts
541548
*/
542549
mergePodXcconfigFile(projectData: IProjectData, platformData: IPlatformData, opts: IRelease): Promise<void>;
543550
}

lib/project-data.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,19 @@ export class ProjectData implements IProjectData {
1616
*/
1717
private static PROJECT_TYPES: IProjectType[] = [
1818
{
19-
type: "Pure JavaScript",
19+
type: constants.ProjectTypes.JsFlavorName,
2020
isDefaultProjectType: true
2121
},
2222
{
23-
type: constants.NgFlavorName,
23+
type: constants.ProjectTypes.NgFlavorName,
2424
requiredDependencies: ["@angular/core", "nativescript-angular"]
2525
},
2626
{
27-
type: constants.VueFlavorName,
27+
type: constants.ProjectTypes.VueFlavorName,
2828
requiredDependencies: ["nativescript-vue"]
2929
},
3030
{
31-
type: "Pure TypeScript",
31+
type: constants.ProjectTypes.TsFlavorName,
3232
requiredDependencies: ["typescript", "nativescript-dev-typescript"]
3333
}
3434
];

lib/services/doctor-service.ts

+60-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { EOL } from "os";
22
import * as path from "path";
33
import * as helpers from "../common/helpers";
4-
import { TrackActionNames } from "../constants";
4+
import { TrackActionNames, NODE_MODULES_FOLDER_NAME, TNS_CORE_MODULES_NAME } from "../constants";
55
import { doctor, constants } from "nativescript-doctor";
66

7-
class DoctorService implements IDoctorService {
7+
export class DoctorService implements IDoctorService {
88
private static DarwinSetupScriptLocation = path.join(__dirname, "..", "..", "setup", "mac-startup-shell-script.sh");
99
private static WindowsSetupScriptExecutable = "powershell.exe";
1010
private static WindowsSetupScriptArguments = ["start-process", "-FilePath", "PowerShell.exe", "-NoNewWindow", "-Wait", "-ArgumentList", '"-NoProfile -ExecutionPolicy Bypass -Command iex ((new-object net.webclient).DownloadString(\'https://www.nativescript.org/setup/win\'))"'];
@@ -14,10 +14,12 @@ class DoctorService implements IDoctorService {
1414
private $logger: ILogger,
1515
private $childProcess: IChildProcess,
1616
private $injector: IInjector,
17+
private $projectDataService: IProjectDataService,
18+
private $fs: IFileSystem,
1719
private $terminalSpinnerService: ITerminalSpinnerService,
1820
private $versionsService: IVersionsService) { }
1921

20-
public async printWarnings(configOptions?: { trackResult: boolean , projectDir?: string, runtimeVersion?: string, options?: IOptions }): Promise<void> {
22+
public async printWarnings(configOptions?: { trackResult: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions }): Promise<void> {
2123
const infos = await this.$terminalSpinnerService.execute<NativeScriptDoctor.IInfo[]>({
2224
text: `Getting environment information ${EOL}`
2325
}, () => doctor.getInfos({ projectDir: configOptions && configOptions.projectDir, androidRuntimeVersion: configOptions && configOptions.runtimeVersion }));
@@ -48,6 +50,8 @@ class DoctorService implements IDoctorService {
4850
this.$logger.error("Cannot get the latest versions information from npm. Please try again later.");
4951
}
5052

53+
this.checkForDeprecatedShortImportsInAppDir(configOptions.projectDir);
54+
5155
await this.$injector.resolve<IPlatformEnvironmentRequirements>("platformEnvironmentRequirements").checkEnvironmentRequirements({
5256
platform: null,
5357
projectDir: configOptions && configOptions.projectDir,
@@ -113,6 +117,59 @@ class DoctorService implements IDoctorService {
113117
return !hasWarnings;
114118
}
115119

120+
public checkForDeprecatedShortImportsInAppDir(projectDir: string): void {
121+
if (projectDir) {
122+
try {
123+
const files = this.$projectDataService.getAppExecutableFiles(projectDir);
124+
const shortImports = this.getDeprecatedShortImportsInFiles(files, projectDir);
125+
if (shortImports.length) {
126+
this.$logger.printMarkdown("Detected short imports in your application. Please note that `short imports are deprecated` since NativeScript 5.2.0. More information can be found in this blogpost https://www.nativescript.org/blog/say-goodbye-to-short-imports-in-nativescript");
127+
shortImports.forEach(shortImport => {
128+
this.$logger.printMarkdown(`In file \`${shortImport.file}\` line \`${shortImport.line}\` is short import. Add \`tns-core-modules/\` in front of the required/imported module.`);
129+
});
130+
}
131+
} catch (err) {
132+
this.$logger.trace(`Unable to validate if project has short imports. Error is`, err);
133+
}
134+
}
135+
}
136+
137+
protected getDeprecatedShortImportsInFiles(files: string[], projectDir: string): { file: string, line: string }[] {
138+
const shortImportRegExps = this.getShortImportRegExps(projectDir);
139+
const shortImports: { file: string, line: string }[] = [];
140+
141+
for (const file of files) {
142+
const fileContent = this.$fs.readText(file);
143+
const strippedComments = helpers.stripComments(fileContent);
144+
const linesWithRequireStatements = strippedComments
145+
.split(/\r?\n/)
146+
.filter(line => /\btns-core-modules\b/.exec(line) === null && (/\bimport\b/.exec(line) || /\brequire\b/.exec(line)));
147+
148+
for (const line of linesWithRequireStatements) {
149+
for (const regExp of shortImportRegExps) {
150+
const matches = line.match(regExp);
151+
152+
if (matches && matches.length) {
153+
shortImports.push({ file, line });
154+
break;
155+
}
156+
}
157+
}
158+
}
159+
160+
return shortImports;
161+
}
162+
163+
private getShortImportRegExps(projectDir: string): RegExp[] {
164+
const pathToTnsCoreModules = path.join(projectDir, NODE_MODULES_FOLDER_NAME, TNS_CORE_MODULES_NAME);
165+
const contents = this.$fs.readDirectory(pathToTnsCoreModules)
166+
.filter(entry => this.$fs.getFsStats(path.join(pathToTnsCoreModules, entry)).isDirectory());
167+
168+
const regExps = contents.map(c => new RegExp(`[\"\']${c}[\"\'/]`, "g"));
169+
170+
return regExps;
171+
}
172+
116173
private async runSetupScriptCore(executablePath: string, setupScriptArgs: string[]): Promise<ISpawnResult> {
117174
return this.$childProcess.spawnFromEvent(executablePath, setupScriptArgs, "close", { stdio: "inherit" });
118175
}

lib/services/platform-service.ts

+3
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export class PlatformService extends EventEmitter implements IPlatformService {
2727
private $errors: IErrors,
2828
private $fs: IFileSystem,
2929
private $logger: ILogger,
30+
private $doctorService: IDoctorService,
3031
private $packageInstallationManager: IPackageInstallationManager,
3132
private $platformsData: IPlatformsData,
3233
private $projectDataService: IProjectDataService,
@@ -237,6 +238,8 @@ export class PlatformService extends EventEmitter implements IPlatformService {
237238
await this.cleanDestinationApp(platformInfo);
238239
}
239240

241+
this.$doctorService.checkForDeprecatedShortImportsInAppDir(platformInfo.projectData.projectDir);
242+
240243
await this.preparePlatformCore(
241244
platformInfo.platform,
242245
platformInfo.appFilesUpdaterOptions,

lib/services/project-data-service.ts

+27-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as path from "path";
22
import { ProjectData } from "../project-data";
33
import { exported } from "../common/decorators";
4-
import { NATIVESCRIPT_PROPS_INTERNAL_DELIMITER, AssetConstants, SRC_DIR, RESOURCES_DIR, MAIN_DIR, CLI_RESOURCES_DIR_NAME } from "../constants";
4+
import { NATIVESCRIPT_PROPS_INTERNAL_DELIMITER, AssetConstants, SRC_DIR, RESOURCES_DIR, MAIN_DIR, CLI_RESOURCES_DIR_NAME, ProjectTypes } from "../constants";
55

66
interface IProjectFileData {
77
projectData: any;
@@ -116,6 +116,32 @@ export class ProjectDataService implements IProjectDataService {
116116
};
117117
}
118118

119+
public getAppExecutableFiles(projectDir: string): string[] {
120+
const projectData = this.getProjectData(projectDir);
121+
122+
let supportedFileExtension = ".js";
123+
if (projectData.projectType === ProjectTypes.NgFlavorName || projectData.projectType === ProjectTypes.TsFlavorName) {
124+
supportedFileExtension = ".ts";
125+
}
126+
127+
const files = this.$fs.enumerateFilesInDirectorySync(
128+
projectData.appDirectoryPath,
129+
(filePath, fstat) => {
130+
if (filePath.indexOf(projectData.appResourcesDirectoryPath) !== -1) {
131+
return false;
132+
}
133+
134+
if (fstat.isDirectory()) {
135+
return true;
136+
}
137+
138+
return path.extname(filePath) === supportedFileExtension;
139+
}
140+
);
141+
142+
return files;
143+
}
144+
119145
private getImageDefinitions(): IImageDefinitionsStructure {
120146
const pathToImageDefinitions = path.join(__dirname, "..", "..", CLI_RESOURCES_DIR_NAME, AssetConstants.assets, AssetConstants.imageDefinitionsFileName);
121147
const imageDefinitions = this.$fs.readJson(pathToImageDefinitions);

test/npm-support.ts

+3
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ function createTestInjector(): IInjector {
107107
extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise<void> => undefined
108108
});
109109
testInjector.register("usbLiveSyncService", () => ({}));
110+
testInjector.register("doctorService", {
111+
checkForDeprecatedShortImportsInAppDir: (projectDir: string): void => undefined
112+
});
110113

111114
return testInjector;
112115
}

test/platform-commands.ts

+3
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,9 @@ function createTestInjector() {
176176
trackOptions: () => Promise.resolve(null)
177177
});
178178
testInjector.register("usbLiveSyncService", ({}));
179+
testInjector.register("doctorService", {
180+
checkForDeprecatedShortImportsInAppDir: (projectDir: string): void => undefined
181+
});
179182

180183
return testInjector;
181184
}

test/platform-service.ts

+3
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ function createTestInjector() {
118118
}
119119
});
120120
testInjector.register("usbLiveSyncService", () => ({}));
121+
testInjector.register("doctorService", {
122+
checkForDeprecatedShortImportsInAppDir: (projectDir: string): void => undefined
123+
});
121124

122125
return testInjector;
123126
}

0 commit comments

Comments
 (0)