Skip to content

Commit f22313c

Browse files
rosen-vladimirovrosen-vladimirov
committed
fix: warnings for short imports should not be shown for minified code
In some cases when there's minified code, CLI prints warning for short imports, while in fact there's no such thing in the code. Fix the regular exepression to be more strict. Also, instead of iterrating over each regular expression for core-modules directories, join them in a single RegExp. This improves the worst case scenario (i.e. when there are short imports for `xml` of `tns-core-modules`) with around 40%.
1 parent 33e5862 commit f22313c

File tree

2 files changed

+62
-14
lines changed

2 files changed

+62
-14
lines changed

‎lib/services/doctor-service.ts‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export class DoctorService implements IDoctorService {
135135
}
136136

137137
protected getDeprecatedShortImportsInFiles(files: string[], projectDir: string): { file: string, line: string }[] {
138-
const shortImportRegExps = this.getShortImportRegExps(projectDir);
138+
const shortImportRegExp = this.getShortImportRegExp(projectDir);
139139
const shortImports: { file: string, line: string }[] = [];
140140

141141
for (const file of files) {
@@ -144,32 +144,40 @@ export class DoctorService implements IDoctorService {
144144
const linesToCheck = _.flatten(strippedComments
145145
.split(/\r?\n/)
146146
.map(line => line.split(";")));
147+
147148
const linesWithRequireStatements = linesToCheck
148149
.filter(line => /\btns-core-modules\b/.exec(line) === null && (/\bimport\b/.exec(line) || /\brequire\b/.exec(line)));
149150

150151
for (const line of linesWithRequireStatements) {
151-
for (const regExp of shortImportRegExps) {
152-
const matches = line.match(regExp);
152+
const matches = line.match(shortImportRegExp);
153153

154-
if (matches && matches.length) {
155-
shortImports.push({ file, line });
156-
break;
157-
}
154+
if (matches && matches.length) {
155+
shortImports.push({ file, line });
158156
}
159157
}
160158
}
161159

162160
return shortImports;
163161
}
164162

165-
private getShortImportRegExps(projectDir: string): RegExp[] {
163+
private getShortImportRegExp(projectDir: string): RegExp {
166164
const pathToTnsCoreModules = path.join(projectDir, NODE_MODULES_FOLDER_NAME, TNS_CORE_MODULES_NAME);
167-
const contents = this.$fs.readDirectory(pathToTnsCoreModules)
165+
const coreModulesSubDirs = this.$fs.readDirectory(pathToTnsCoreModules)
168166
.filter(entry => this.$fs.getFsStats(path.join(pathToTnsCoreModules, entry)).isDirectory());
169167

170-
const regExps = contents.map(c => new RegExp(`[\"\']${c}[\"\'/]`, "g"));
168+
const stringRegularExpressionsPerDir = coreModulesSubDirs.map(c => {
169+
// require("text");
170+
// require("text/smth");
171+
// require( "text/smth");
172+
// require( "text/smth" );
173+
// import * as text from "text";
174+
// import { a } from "text";
175+
// import {a } from "text/abc"
176+
const subDirPart = `[\"\']${c}[\"\'/]`;
177+
return `(\\brequire\\s*?\\(\\s*?${subDirPart})|(\\bimport\\b.*?from\\s*?${subDirPart})`;
178+
});
171179

172-
return regExps;
180+
return new RegExp(stringRegularExpressionsPerDir.join("|"), "g");
173181
}
174182

175183
private async runSetupScriptCore(executablePath: string, setupScriptArgs: string[]): Promise<ISpawnResult> {

‎test/services/doctor-service.ts‎

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ describe("doctorService", () => {
4141
describe("checkForDeprecatedShortImportsInAppDir", () => {
4242
const tnsCoreModulesDirs = [
4343
"application",
44-
"data"
44+
"data",
45+
"text"
4546
];
4647

4748
const testData: { filesContents: IStringDictionary, expectedShortImports: any[] }[] = [
@@ -179,15 +180,54 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl
179180
},
180181
expectedShortImports: []
181182
},
183+
{
184+
filesContents: {
185+
// minified code that has both require and some of the tns-core-modules subdirs (i.e. text)
186+
file1: `o.cache&&(r+="&cache="+u(o.cache)),t=["<!DOCTYPE html>","<html>","<head>",'<meta charset="UTF-8" />','<script type="text/javascript">'," var UWA = {hosts:"+n(e.hosts)+"},",' curl = {apiName: "require"};`
187+
},
188+
expectedShortImports: []
189+
},
190+
{
191+
filesContents: {
192+
// spaces around require
193+
file1: 'const application = require ( "application" ); console.log("application");',
194+
},
195+
expectedShortImports: [
196+
{ file: "file1", line: 'const application = require ( "application" )' }
197+
]
198+
},
199+
{
200+
filesContents: {
201+
// spaces around require
202+
file1: 'const application = require ( "tns-core-modules/application" ); console.log("application");',
203+
},
204+
expectedShortImports: []
205+
},
206+
{
207+
filesContents: {
208+
// spaces in import line
209+
file1: "import { run } from 'application' ;",
210+
},
211+
expectedShortImports: [
212+
{ file: "file1", line: "import { run } from 'application' " }
213+
]
214+
},
215+
{
216+
filesContents: {
217+
// spaces in import line
218+
file1: "import { run } from 'tns-core-modules/application' ;",
219+
},
220+
expectedShortImports: []
221+
},
182222
{
183223
// Incorrect behavior, currently by design
184224
// In case you have a multiline string and one of the lines matches our RegExp we'll detect it as short import
185225
filesContents: {
186-
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`)',
226+
file1: 'const _ = require("lodash");const application = require("application");console.log("application");console.log(`this is line\nyou should import some long words here require("application") module and other words here`)',
187227
},
188228
expectedShortImports: [
189229
{ file: "file1", line: 'const application = require("application")' },
190-
{ file: "file1", line: 'you should import some long words here "application" module and other words here`)' },
230+
{ file: "file1", line: 'you should import some long words here require("application") module and other words here`)' },
191231
]
192232
},
193233
];

0 commit comments

Comments
 (0)