Skip to content

Commit 2b14454

Browse files
rosen-vladimirovrosen-vladimirov
authored
Merge pull request #4454 from NativeScript/vladimirov/fix-shorts-2
fix: warnings for short imports should not be shown for minified code
2 parents 33e5862 + f22313c commit 2b14454

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)