Skip to content
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.

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

feat: worker based linting #386

Draft
wants to merge 5 commits into
base: component-context-info-origin
Choose a base branch
from
Draft
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
linter per project
  • Loading branch information
lifeart committed Apr 28, 2022
commit 5f8a7ec387a8f4763ae816e7ac5c0d405a5a1026
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -89,7 +89,7 @@
"ts-loader": "^9.2.3",
"ts-node": "^8.10.2",
"typescript": "^4.6.3",
"webpack": "^5.44.0",
"webpack": "^5.72.0",
"webpack-bundle-analyzer": "^4.5.0",
"webpack-cli": "^4.7.2"
},
@@ -109,6 +109,7 @@
"clean": "rimraf lib/",
"build:bundle:node": "webpack --mode production",
"build:bundle:worker": "webpack --mode production && node ./fix-worker-bundle.js",
"build:bundle:linter-thread": "webpack --mode production",
"compile": "tsc --skipLibCheck -p .",
"lint": "eslint \"./{src,test}/**/*.ts\"",
"prepublish": "yarn clean && yarn compile",
15 changes: 0 additions & 15 deletions src/builtin-addons/core/code-actions/template-lint-fixes.ts
Original file line number Diff line number Diff line change
@@ -2,17 +2,8 @@ import { CodeActionFunctionParams } from '../../../utils/addon-api';
import { Command, CodeAction, WorkspaceEdit, CodeActionKind, TextEdit, Diagnostic } from 'vscode-languageserver/node';
import { toLSRange } from '../../../estree-utils';
import BaseCodeActionProvider, { INodeSelectionInfo } from './base';
import { logError } from '../../../utils/logger';
import { TextDocument } from 'vscode-languageserver-textdocument';

function setCwd(cwd: string) {
try {
process.chdir(cwd);
} catch (err) {
logError(err);
}
}

export default class TemplateLintFixesCodeAction extends BaseCodeActionProvider {
async fixTemplateLintIssues(issues: Diagnostic[], params: CodeActionFunctionParams, meta: INodeSelectionInfo): Promise<Array<CodeAction | null>> {
if (!this.server.templateLinter.isEnabled) {
@@ -25,11 +16,7 @@ export default class TemplateLintFixesCodeAction extends BaseCodeActionProvider
return [null];
}

const cwd = process.cwd();

try {
setCwd(this.project.root);

const fixes = issues.map(async (issue): Promise<null | CodeAction> => {
const result = await this.server.templateLinter.fix(TextDocument.create(params.textDocument.uri, 'handlebars', 1, meta.selection || ''));

@@ -50,8 +37,6 @@ export default class TemplateLintFixesCodeAction extends BaseCodeActionProvider
return resolvedFixes;
} catch (e) {
return [];
} finally {
setCwd(cwd);
}
}
public async onCodeAction(_: string, params: CodeActionFunctionParams): Promise<(Command | CodeAction)[] | undefined | null> {
30 changes: 7 additions & 23 deletions src/linter-thread.ts
Original file line number Diff line number Diff line change
@@ -127,32 +127,18 @@ async function getLinterInstance(msg: LinterMessage) {
const LinterKlass = linters.get(msg.projectRoot);

if (LinterKlass) {
const cwd = process.cwd();

setCwd(msg.projectRoot);

try {
instances.set(msg.projectRoot, new LinterKlass());
} catch (e) {
// EOL
}

setCwd(cwd);
}
}
}

return instances.get(msg.projectRoot);
}

function setCwd(cwd: string) {
try {
process.chdir(cwd);
} catch (err) {
// EOL
}
}

async function fixDocument(message: LinterMessage): Promise<[null | Error, { isFixed: boolean; output?: string }]> {
try {
await linkLinterToProject(message);
@@ -217,13 +203,11 @@ async function lintDocument(message: LinterMessage): Promise<[null | Error, Diag
try {
const results = await Promise.all(
sources.map(async (source) => {
const errors = await Promise.resolve(
(linter as Linter).verify({
source,
moduleId: URI.parse(message.uri).fsPath,
filePath: URI.parse(message.uri).fsPath,
})
);
const errors = await (linter as Linter).verify({
source,
moduleId: URI.parse(message.uri).fsPath,
filePath: URI.parse(message.uri).fsPath,
});

return errors.map((error: TemplateLinterError) => toDiagnostic(source, error));
})
@@ -252,7 +236,7 @@ parentPort?.on('message', async (message: LinterMessage) => {
} catch (e) {
parentPort?.postMessage({
id: message.id,
error: e.message,
error: e,
diagnostics: [],
});
}
@@ -269,7 +253,7 @@ parentPort?.on('message', async (message: LinterMessage) => {
} catch (e) {
parentPort?.postMessage({
id: message.id,
error: e.message,
error: e,
isFixed: false,
output: '',
});
117 changes: 76 additions & 41 deletions src/template-linter.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { Diagnostic, Files } from 'vscode-languageserver/node';
import { TextDocument } from 'vscode-languageserver-textdocument';
import { getExtension } from './utils/file-extension';
import { log, logDebugInfo } from './utils/logger';
import { log, logError, logInfo } from './utils/logger';
import { Worker } from 'worker_threads';
import Server from './server';
import { Project } from './project';
import { getRequireSupport } from './utils/layout-helpers';
import { extensionsToLint, LinterMessage } from './linter-thread';
import * as path from 'path';

type FindUp = (name: string, opts: { cwd: string; type: string }) => Promise<string | undefined>;

@@ -47,35 +48,23 @@ type QItem = {
tId: number;
};

export default class TemplateLinter {
private _linterCache = new Map<Project, string>();
private _isEnabled = true;
private _findUp: FindUp;
private worker: Worker;
private _qID = 0;

constructor(private server: Server) {
if (this.server.options.type === 'worker') {
this.disable();
}
}

private workerQueue: QItem[] = [];

initWorker() {
if (this.worker) {
this.workerQueue = [];
}
class WorkerWrapper {
worker: Worker;
queue: QItem[];
constructor(worker: Worker) {
this.worker = worker;
this.queue = [];
worker.on('message', (message: WorkerMessage) => {
const q = this.queue.find((q) => q.id === message.id);

this.worker = new Worker('./linter-thread.ts');
this.worker.on('message', (message: WorkerMessage) => {
const q = this.workerQueue.find((q) => q.id === message.id);
logInfo(`Message for ${message.id}: ${JSON.stringify(message)}`);

if (q) {
this.workerQueue = this.workerQueue.filter((q) => q.id !== message.id);
this.queue = this.queue.filter((q) => q.id !== message.id);
clearTimeout(q.tId);

if (message.error !== null) {
logInfo(message.error);
q.reject(message.error);
} else {
if ('diagnostics' in message) {
@@ -86,13 +75,56 @@ export default class TemplateLinter {
}
}
});
this.worker.on('error', () => {
this.initWorker();
});
this.worker.on('exit', () => {
this.initWorker();
worker.on('error', (e) => {
logError(e as Error & { stack: string });
});
}
addToQueue(q: QItem, fn: () => LinterMessage) {
if (this.queue.length < 100) {
q.tId = setTimeout(() => {
this.queue = this.queue.filter((q) => q !== q);
logInfo(`Timeout for ${q.id}`);
}, 10000) as unknown as number;
this.queue.push(q);
logInfo(`Adding to queue ${q.id}`);
this.worker.postMessage(fn());
}
}
}

export default class TemplateLinter {
private _linterCache = new Map<Project, string>();
private _isEnabled = true;
private _findUp: FindUp;
private workers: WeakMap<Project, WorkerWrapper> = new WeakMap();
private _qID = 0;

constructor(private server: Server) {
if (this.server.options.type === 'worker') {
this.disable();
}
}

initWorker(project: Project): WorkerWrapper | undefined {
try {
if (!this.workers.has(project)) {
const worker = new Worker(path.join(__dirname, './linter-thread.js'), {
workerData: {
cwd: project.root,
},
});
const wrapper = new WorkerWrapper(worker);

this.workers.set(project, wrapper);
}

return this.workers.get(project);
} catch (e) {
logError(e);

return undefined;
}
}

disable() {
this._isEnabled = false;
@@ -139,11 +171,17 @@ export default class TemplateLinter {
return;
}

const wrapper = await this.initWorker(project);

if (!wrapper) {
return;
}

const p: Promise<FixOutput> = new Promise((resolve, reject) => {
const id = this.qID();
const ref = { id, resolve, reject, tId: -1 };

this.addToQueue(ref, () => {
wrapper.addToQueue(ref, () => {
const msg: LinterMessage = {
id,
action: 'verifyAndFix',
@@ -179,11 +217,17 @@ export default class TemplateLinter {
return;
}

const wrapper = await this.initWorker(project);

if (!wrapper) {
return;
}

const p: Promise<Diagnostic[]> = new Promise((resolve, reject) => {
const id = this.qID();
const ref = { id, resolve, reject, tId: -1 };

this.addToQueue(ref, () => {
wrapper.addToQueue(ref, () => {
const msg: LinterMessage = {
id,
action: 'verify',
@@ -202,20 +246,11 @@ export default class TemplateLinter {

return diagnostics;
} catch (e) {
logDebugInfo(e);
logInfo(e);

return [];
}
}
addToQueue(q: QItem, fn: () => LinterMessage) {
if (this.workerQueue.length < 100) {
q.tId = setTimeout(() => {
this.workerQueue = this.workerQueue.filter((q) => q !== q);
}, 10000) as unknown as number;
this.workerQueue.push(q);
this.worker.postMessage(fn());
}
}
async getFindUp(): Promise<FindUp> {
if (!this._findUp) {
const { findUp } = await eval(`import('find-up')`);
40 changes: 40 additions & 0 deletions webpack.config.js
Original file line number Diff line number Diff line change
@@ -48,6 +48,41 @@ const nodeBundleConfig = {
},
};

const linterWorkerBundle = /** @type WebpackConfig */ {
mode: 'production',
target: 'node',
entry: {
'linter-thread': './src/linter-thread.ts',
},
output: {
// the bundle is stored in the 'dist' folder (check package.json), 📖 -> https://webpack.js.org/configuration/output/
path: path.join(__dirname, 'dist', 'bundled'),
filename: 'linter-thread.js',
libraryTarget: 'commonjs2',
devtoolModuleFilenameTemplate: '../[resource-path]',
},
// devtool: 'source-map',
module: {
rules: [
{
test: /\.ts$/,
exclude: /node_modules/,
use: [
{
loader: 'ts-loader',
},
],
},
],
},
resolve: {
// support reading TypeScript and JavaScript files, 📖 -> https://github.com/TypeStrong/ts-loader
mainFields: ['module', 'main'],
extensions: ['.ts', '.js'], // support ts-files and js-files
},

}

const workerBundleConfig = /** @type WebpackConfig */ {
mode: 'production',
target: 'webworker', // web extensions run in a webworker context
@@ -85,6 +120,7 @@ const workerBundleConfig = /** @type WebpackConfig */ {
// assert: false,
// buffer: false,
browserlist: false,
'worker_threads': false,
'@babel/generator': false,
'@babel/highlight': false,
// '@babel/types': false,
@@ -134,6 +170,10 @@ const configs = [
name: 'build:bundle:worker',
config: workerBundleConfig,
},
{
name: 'build:bundle:linter-thread',
config: linterWorkerBundle
}
];

module.exports = configs.filter(({ name }) => name.startsWith(buildName)).map((e) => e.config);
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
@@ -6680,10 +6680,10 @@ webpack-sources@^3.2.3:
resolved "https://registry.yarnpkg.com/webpack-sources/-/webpack-sources-3.2.3.tgz#2d4daab8451fd4b240cc27055ff6a0c2ccea0cde"
integrity sha512-/DyMEOrDgLKKIG0fmvtz+4dUX/3Ghozwgm6iPp8KRhvn+eQf9+Q7GWxVNMk3+uCPWfdXYC4ExGBckIXdFEfH1w==

webpack@^5.44.0:
version "5.70.0"
resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.70.0.tgz#3461e6287a72b5e6e2f4872700bc8de0d7500e6d"
integrity sha512-ZMWWy8CeuTTjCxbeaQI21xSswseF2oNOwc70QSKNePvmxE7XW36i7vpBMYZFAUHPwQiEbNGCEYIOOlyRbdGmxw==
webpack@^5.72.0:
version "5.72.0"
resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.72.0.tgz#f8bc40d9c6bb489a4b7a8a685101d6022b8b6e28"
integrity sha512-qmSmbspI0Qo5ld49htys8GY9XhS9CGqFoHTsOVAnjBdg0Zn79y135R+k4IR4rKK6+eKaabMhJwiVB7xw0SJu5w==
dependencies:
"@types/eslint-scope" "^3.7.3"
"@types/estree" "^0.0.51"