Skip to content

fix: warnings for short imports should not be shown for minified code #4454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 20, 2019
Merged

fix: warnings for short imports should not be shown for minified code #4454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions lib/services/doctor-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export class DoctorService implements IDoctorService {
}

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

for (const file of files) {
Expand All @@ -144,32 +144,40 @@ export class DoctorService implements IDoctorService {
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) {
for (const regExp of shortImportRegExps) {
const matches = line.match(regExp);
const matches = line.match(shortImportRegExp);

if (matches && matches.length) {
shortImports.push({ file, line });
break;
}
if (matches && matches.length) {
shortImports.push({ file, line });
}
}
}

return shortImports;
}

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

const regExps = contents.map(c => new RegExp(`[\"\']${c}[\"\'/]`, "g"));
const stringRegularExpressionsPerDir = coreModulesSubDirs.map(c => {
// require("text");
// require("text/smth");
// require( "text/smth");
// require( "text/smth" );
// import * as text from "text";
// import { a } from "text";
// import {a } from "text/abc"
const subDirPart = `[\"\']${c}[\"\'/]`;
return `(\\brequire\\s*?\\(\\s*?${subDirPart})|(\\bimport\\b.*?from\\s*?${subDirPart})`;
});

return regExps;
return new RegExp(stringRegularExpressionsPerDir.join("|"), "g");
}

private async runSetupScriptCore(executablePath: string, setupScriptArgs: string[]): Promise<ISpawnResult> {
Expand Down
46 changes: 43 additions & 3 deletions test/services/doctor-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ describe("doctorService", () => {
describe("checkForDeprecatedShortImportsInAppDir", () => {
const tnsCoreModulesDirs = [
"application",
"data"
"data",
"text"
];

const testData: { filesContents: IStringDictionary, expectedShortImports: any[] }[] = [
Expand Down Expand Up @@ -179,15 +180,54 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl
},
expectedShortImports: []
},
{
filesContents: {
// minified code that has both require and some of the tns-core-modules subdirs (i.e. text)
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"};`
},
expectedShortImports: []
},
{
filesContents: {
// spaces around require
file1: 'const application = require ( "application" ); console.log("application");',
},
expectedShortImports: [
{ file: "file1", line: 'const application = require ( "application" )' }
]
},
{
filesContents: {
// spaces around require
file1: 'const application = require ( "tns-core-modules/application" ); console.log("application");',
},
expectedShortImports: []
},
{
filesContents: {
// spaces in import line
file1: "import { run } from 'application' ;",
},
expectedShortImports: [
{ file: "file1", line: "import { run } from 'application' " }
]
},
{
filesContents: {
// spaces in import line
file1: "import { run } from 'tns-core-modules/application' ;",
},
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`)',
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`)',
},
expectedShortImports: [
{ file: "file1", line: 'const application = require("application")' },
{ file: "file1", line: 'you should import some long words here "application" module and other words here`)' },
{ file: "file1", line: 'you should import some long words here require("application") module and other words here`)' },
]
},
];
Expand Down