Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Refactored to perform linting/fixing on async worker task #158

Merged
merged 3 commits into from
Apr 24, 2017
Merged

Refactored to perform linting/fixing on async worker task #158

merged 3 commits into from
Apr 24, 2017

Conversation

Xapphire13
Copy link
Contributor

@Xapphire13 Xapphire13 commented Apr 20, 2017

Loosely based on @Arcanemagus's work on eslint/task-api

Caveats

  • Specs now run a tad slower since each test needs to instantiate a task proccess

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I see so far, I like your solution to the "has the worker been started" problem!

this.worker = new Task(require.resolve('./worker.js'));
idleCallbacks.delete(createWorkerCallback);
});
idleCallbacks.add(createWorkerCallback);
},

deactivate() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to terminate your worker here in deactivate().

lib/main.js Outdated
if (filePath === null || filePath === undefined) {
return null;
}
const text = textEditor.getText();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this below the worker initialization check so the "conflict window" is as small as possible.

lib/worker.js Outdated
}
} else {
const { emitKey, jobType, content, filePath } = message.message;
const options = jobType === 'fix' ? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just put this all on one line, there's only the one option.

lib/worker.js Outdated

function loadDefaultTSLint() {
if (!tslintDef) {
tslintDef = require('loophole').allowUnsafeNewFunction(() =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need the loophole here? I'm not sure that Atom's CSP actually applies to Task processes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea (since this part was simply moving the methods). I shall try, then get back to you =]

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants