Skip to content

Commit

Permalink
Eslint integration (#492)
Browse files Browse the repository at this point in the history
* Implemented eslint inside tools

* Removed eslint feature check

* Removed test for ESLint

* Implemented new class LintValidator, now using eslintrc from visual by default

* Fixed missing options in package command

* Fixed audit error

* fixed tests

* Updated packages
  • Loading branch information
AleksSavelev authored Jan 8, 2024
1 parent 58bf327 commit b100e9d
Show file tree
Hide file tree
Showing 11 changed files with 265 additions and 134 deletions.
8 changes: 8 additions & 0 deletions bin/pbiviz.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ pbiviz
CommandManager.installCert();
});

pbiviz
.command('lint')
.option('--fix', 'Enable autofixing of lint errors')
.action(options => {
CommandManager.lint({ ...options, verbose: true }, rootPath);
});

pbiviz
.command('start')
.usage('[options]')
Expand All @@ -92,6 +99,7 @@ pbiviz
.option('--skip-api', "Skips powerbi-visuals-api verifying")
.option('-l, --all-locales', "Keeps all locale files in the package. By default only used inside stringResources folder locales are included.")
.option('-v, --verbose', "Enables verbose logging")
.option('--fix', 'Enable autofixing of lint errors')
.option('-p, --pbiviz-file <pbiviz-file>', "Path to pbiviz.json file (useful for debugging)", pbivizFile)
.addOption(new Option('-c, --compression <compressionLevel>', "Enables compression of visual package")
.choices(['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'])
Expand Down
193 changes: 108 additions & 85 deletions package-lock.json

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
},
"homepage": "https://github.com/Microsoft/PowerBI-visuals-tools#readme",
"dependencies": {
"@typescript-eslint/parser": "^6.12.0",
"@typescript-eslint/parser": "^6.17.0",
"assert": "^2.1.0",
"async": "^3.2.5",
"browserify-zlib": "^0.2.0",
Expand All @@ -41,16 +41,16 @@
"constants-browserify": "^1.0.0",
"crypto-browserify": "^3.12.0",
"css-loader": "^6.8.1",
"domain-browser": "^5.4.0",
"domain-browser": "^5.7.0",
"events": "^3.3.0",
"extra-watch-webpack-plugin": "^1.0.3",
"fs-extra": "^11.1.1",
"fs-extra": "^11.2.0",
"https-browserify": "^1.0.0",
"inline-source-map": "^0.6.2",
"json-loader": "0.5.7",
"jszip": "^3.10.1",
"less": "^4.2.0",
"less-loader": "^11.1.3",
"less-loader": "^11.1.4",
"lodash.clonedeep": "4.5.0",
"lodash.defaults": "4.2.0",
"lodash.isequal": "4.5.0",
Expand All @@ -62,11 +62,11 @@
"process": "^0.11.10",
"punycode": "^2.3.1",
"querystring-es3": "^0.2.1",
"readable-stream": "^4.4.2",
"readable-stream": "^4.5.2",
"stream-browserify": "^3.0.0",
"stream-http": "^3.2.0",
"string_decoder": "^1.3.0",
"terser-webpack-plugin": "^5.3.9",
"terser-webpack-plugin": "^5.3.10",
"timers-browserify": "^2.0.12",
"ts-loader": "^9.5.1",
"tty-browserify": "^0.0.1",
Expand All @@ -79,8 +79,8 @@
"webpack-dev-server": "^4.15.1"
},
"devDependencies": {
"@typescript-eslint/eslint-plugin": "^6.12.0",
"eslint": "^8.54.0",
"@typescript-eslint/eslint-plugin": "^6.17.0",
"eslint": "^8.56.0",
"eslint-plugin-powerbi-visuals": "^0.8.1",
"jasmine": "5.1.0",
"jasmine-spec-reporter": "7.0.0",
Expand Down
8 changes: 1 addition & 7 deletions spec/unit/FeatureManagerSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const config = readJsonFromRoot('config.json');

describe("Features", () => {
describe("Visual", () => {
const { APIVersion, ESLint, VisualVersion } = features;
const { APIVersion, VisualVersion } = features;
it("Should support API Version", () => {
const Visual = {
doesAPIVersionMatch: (minVersion) => {
Expand All @@ -46,12 +46,6 @@ describe("Features", () => {
expect(APIVersion.isSupported(Visual)).toBeTrue;
});

it("Should support ESLint", () => {
const Visual = {
doesESLlintSupported: () => true
}
expect(ESLint.isSupported(Visual)).toBeTrue;
});

it("Should support Version", () => {
const Visual = {
Expand Down
22 changes: 17 additions & 5 deletions src/CommandManager.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

import { createCertificate } from './CertificateTools.js';
import ConsoleWriter from './ConsoleWriter.js';
import VisualManager from './VisualManager.js';
import VisualManager, { LintOptions, GenerateOptions } from './VisualManager.js';
import { WebpackOptions } from './WebPackWrap.js';

interface StartOptions {
Expand All @@ -22,6 +22,7 @@ interface PackageOptions {
skipApi: boolean;
allLocales: boolean;
verbose: boolean;
fix: boolean;
pbivizFile: string;
}

Expand Down Expand Up @@ -53,6 +54,13 @@ export default class CommandManager {
.initializeWebpack(webpackOptions)
visualManager.startWebpackServer(options.drop)
}

public static async lint(options: LintOptions, rootPath: string) {
const visualManager = new VisualManager(rootPath)
await visualManager
.prepareVisual()
.runLintValidation(options)
}

public static async package(options: PackageOptions, rootPath: string) {
if (!options.pbiviz && !options.resources) {
Expand All @@ -72,15 +80,19 @@ export default class CommandManager {
allLocales: options.allLocales,
pbivizFile: options.pbivizFile,
}
new VisualManager(rootPath)
.prepareVisual(options.pbivizFile)
.validateVisual(options.verbose)
const lintOptions: LintOptions = {
verbose: options.verbose,
fix: options.fix,
}
const visual = new VisualManager(rootPath).prepareVisual(options.pbivizFile)
await visual.runLintValidation(lintOptions)
visual.validateVisual(options.verbose)
.initializeWebpack(webpackOptions)
.then(visualManager => visualManager.generatePackage(options.verbose))
}

public static new({ force, template }: NewOptions, name: string, rootPath: string) {
const generateOptions = {
const generateOptions: GenerateOptions = {
force: force,
template: template
};
Expand Down
87 changes: 87 additions & 0 deletions src/LintValidator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { ESLint } from "eslint";

import ConsoleWriter from "./ConsoleWriter.js";
import { LintOptions } from "./VisualManager.js";
import { fileExists, getRootPath } from "./utils.js";

export class LintValidator {

private visualPath: string;
private rootPath: string;
private config: ESLint.Options;
private defaultConfig: ESLint.Options;
private linterInstance: ESLint;

constructor(fix: boolean = false) {
this.visualPath = process.cwd()
this.rootPath = getRootPath();
this.prepareConfig(fix);
this.linterInstance = new ESLint(this.config);
}

/**
* Runs lint validation in the visual folder
*/
public async runLintValidation({ verbose, fix }: LintOptions) {
ConsoleWriter.info("Running lint check...");
// By default it will lint all files in the src of current working directory, but some files can be excluded in .eslintignore
const results = await this.linterInstance.lintFiles("src/**/*");

if (fix) {
await this.fixErrors(results);
}
await this.outputResults(results, verbose);
ConsoleWriter.info("Lint check completed.");
}

private async fixErrors(results: ESLint.LintResult[]) {
ConsoleWriter.info("Lint fixing errors...");
await ESLint.outputFixes(results);
}

private async outputResults(results: ESLint.LintResult[], verbose: boolean) {
if (verbose) {
const formatter = await this.linterInstance.loadFormatter("stylish");
const formattedResults = await formatter.format(results);
console.log(formattedResults)
} else {
const filteredResults = ESLint.getErrorResults(results);
// get total amount of errors and warnings in all elements of filteredResults
const totalErrors = filteredResults.reduce((acc, curr) => acc + curr.errorCount, 0);
const totalWarnings = filteredResults.reduce((acc, curr) => acc + curr.warningCount, 0);
if (totalErrors > 0 || totalWarnings > 0) {
ConsoleWriter.error(`Linter found ${totalErrors} errors and ${totalWarnings} warnings. Run with --verbose flag to see details.`)
}
}
}

private prepareConfig(fix: boolean) {
this.defaultConfig = {
overrideConfig: {
env: {
browser: true,
es6: true,
es2022: true
},
plugins: [
"powerbi-visuals"
],
extends: [
"plugin:powerbi-visuals/recommended"
]
},
extensions: [".ts", ".tsx"],
resolvePluginsRelativeTo: this.rootPath,
useEslintrc: false,
fix
}

const eslintrcExtensions = ['.json', '.js', '.ts']
if (eslintrcExtensions.some(el => fileExists(this.visualPath, `.eslintrc${el}`))){
this.config = { fix }
} else {
ConsoleWriter.warning("No .eslintrc file found in the visual folder. Using default config.")
this.config = this.defaultConfig
}
}
}
8 changes: 1 addition & 7 deletions src/Visual.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,19 @@ export class Visual {
public visualFeatureType: VisualFeatureType;
private capabilities;
private config;
private packageJSON;
private visualVersion: string;

constructor(capabilities, config, packageJson) {
constructor(capabilities, config) {
this.capabilities = capabilities;
this.config = config;
this.visualFeatureType = this.getVisualFeatureType();
this.packageJSON = packageJson;
this.visualVersion = config.visual.version;
}

public doesAPIVersionMatch(minAPIversion: string) {
return compareVersions(this.config.apiVersion ?? minAPIversion, minAPIversion) !== -1
}

public doesESLlintSupported() {
return Object.entries(this.packageJSON.scripts).some(([, value]) => (<string>value).includes("eslint"))
}

public isVisualVersionValid(length: number) {
return this.visualVersion.split(".").length === length
}
Expand Down
35 changes: 31 additions & 4 deletions src/VisualManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,18 @@ import { Visual } from "./Visual.js";
import { FeatureManager, Logs, Status } from "./FeatureManager.js";
import { Severity, Stage } from "./features/FeatureTypes.js";
import TemplateFetcher from "./TemplateFetcher.js";
import { LintValidator } from "./LintValidator.js";

interface GenerateOptions {
export interface GenerateOptions {
force: boolean;
template: string;
}

export interface LintOptions {
verbose: boolean;
fix: boolean;
}

const globalConfig = readJsonFromRoot('config.json');
const PBIVIZ_FILE = 'pbiviz.json';

Expand Down Expand Up @@ -79,10 +85,14 @@ export default class VisualManager {
return this;
}

public runLintValidation(options: LintOptions) {
const linter = new LintValidator(options.fix);
linter.runLintValidation(options);
}

public createVisualInstance() {
this.capabilities = readJsonFromVisual("capabilities.json", this.basePath);
const packageJSON = readJsonFromVisual("package.json", this.basePath);
this.visual = new Visual(this.capabilities, this.pbivizConfig, packageJSON);
this.visual = new Visual(this.capabilities, this.pbivizConfig);
}

public async initializeWebpack(webpackOptions: WebpackOptions) {
Expand All @@ -104,6 +114,9 @@ export default class VisualManager {
this.compiler.run(callback);
}

/**
* Starts webpack server
*/
public startWebpackServer(generateDropFiles: boolean = false) {
ConsoleWriter.blank();
ConsoleWriter.info('Starting server...');
Expand Down Expand Up @@ -134,6 +147,9 @@ export default class VisualManager {
}
}

/**
* Validates the visual code
*/
public validateVisual(verbose: boolean = false) {
this.featureManager = new FeatureManager()
const { status, logs } = this.featureManager.validate(Stage.PreBuild, this.visual);
Expand All @@ -145,13 +161,19 @@ export default class VisualManager {
return this;
}

/**
* Validates the visual package
*/
public validatePackage() {
const featureManager = new FeatureManager();
const { logs } = featureManager.validate(Stage.PostBuild, this.package);

return logs;
}

/**
* Outputs the results of the validation
*/
public outputResults({ errors, deprecation, warnings, info }: Logs, verbose: boolean) {
const headerMessage = {
error: `Visual doesn't support some features required for all custom visuals:`,
Expand All @@ -171,6 +193,9 @@ export default class VisualManager {
this.outputLogsWithHeadMessage(headerInfoMessage, infoLogs, Severity.Info);
}

/**
* Displays visual info
*/
public displayInfo() {
if (this.pbivizConfig) {
ConsoleWriter.infoTable(this.pbivizConfig);
Expand All @@ -179,8 +204,10 @@ export default class VisualManager {
}
}



/**
* Creates a new visual package
* Creates a new visual
*/
static async createVisual(rootPath: string, visualName: string, generateOptions: GenerateOptions): Promise<VisualManager | void> {
ConsoleWriter.info('Creating new visual');
Expand Down
16 changes: 0 additions & 16 deletions src/features/ESLint.ts

This file was deleted.

Loading

0 comments on commit b100e9d

Please sign in to comment.