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

Add eslintTask to just-scripts #122

Closed
Thisen opened this issue Jun 4, 2019 · 7 comments
Closed

Add eslintTask to just-scripts #122

Thisen opened this issue Jun 4, 2019 · 7 comments

Comments

@Thisen
Copy link
Contributor

Thisen commented Jun 4, 2019

Since TypeScript itself are moving to ESLint, it would be a great addition to add aneslintTask() to just-scripts, similar to tslintTask().

@kenotron
Copy link
Member

kenotron commented Jun 4, 2019

Yes yes yes! Would you want to actually write a task? @Thisen

@Thisen
Copy link
Contributor Author

Thisen commented Jun 4, 2019

Sure thing, I can give it a shot @kenotron.

@Thisen
Copy link
Contributor Author

Thisen commented Jun 5, 2019

@kenotron, I'm not sure how I can contribute with the Rush git policies of this repository. Anything I'm missing?

@Thisen
Copy link
Contributor Author

Thisen commented Jun 5, 2019

My first iteration (not done) looks like this:

import { resolve, logger, resolveCwd, TaskFunction } from 'just-task';
import { exec, encodeArgs } from 'just-scripts-utils';
import fs from 'fs';

export interface EsLintTaskOptions {
  configPath?: string;
  ignorePath?: string;
  fix?: boolean;
}

export function eslintTask(options: EsLintTaskOptions = {}): TaskFunction {
  return function eslint() {
    const eslintCli = resolve('eslint/lib/cli.js');
    const eslintConfigPath = resolveCwd((options && options.configPath) || '.eslintrc.js');

    if (eslintCli && eslintConfigPath && fs.existsSync(eslintConfigPath)) {
      logger.info(`Running ESLint`);
      const eslintIgnorePath = resolveCwd((options && options.ignorePath) || '.eslintignore');

      const eslintArgs = [
        eslintCli,
        ...(eslintConfigPath ? ['--config', eslintConfigPath] : []),
        ...(eslintIgnorePath ? ['--ignore', eslintIgnorePath] : []),
        ...(options.fix ? ['--fix'] : [])
      ];

      const cmd = encodeArgs([process.execPath, eslintCli, ...eslintArgs]).join(' ');
      logger.info(cmd);
      return exec(cmd, { stdout: process.stdout, stderr: process.stderr });
    } else {
      return Promise.resolve();
    }
  };
}

It's on purpose that not every CLI options in ESLint are exposed, since I see that pattern in the other tasks.

Do you just to expose everything or you want something more strict?

Other than that, I chose to have .eslintrc.js as the default - Of course this can be changed aswell.

@Thisen
Copy link
Contributor Author

Thisen commented Jun 12, 2019

Hi @kenotron,

I've finished a ESLint task, that I'm using in my current project. It's written in JS, since it's used by our just-scripts task directly, but it's easily migrated to TypeScript and more standardized:

function eslintTask(options) {
  return function eslint() {
    const eslintCmd = resolve("eslint/bin/eslint.js");
    const eslintConfigPath = (options && options.configPath) || resolveCwd(".eslintrc");
    if (eslintCmd && eslintConfigPath && fs.existsSync(eslintConfigPath)) {
      const eslintIgnorePath = (options && options.ignorePath) || resolveCwd(".eslintignore");

      const eslintArgs = [
        eslintCmd,
        ...(options && options.files ? options.files : ["."]),
        ...(eslintConfigPath ? ["--config", eslintConfigPath, "--no-eslintrc"] : []),
        ...[
          "--ignore-path",
          eslintIgnorePath ? eslintIgnorePath : path.resolve(__dirname, "..", "..", ".eslintignore")
        ],
        ...(options && options.fix ? ["--fix"] : []),
        ...["--max-warnings", "0"],
        "--color"
      ];

      logger.info(encodeArgs(eslintArgs).join(" "));
      return spawn(process.execPath, eslintArgs, { stdio: "inherit" });
    } else {
      return Promise.resolve();
    }
  };
}

Let me know what you think in terms of interface.

@kenotron
Copy link
Member

@Thisen - I recently swapped to lerna + yarn. This should help! I don't have a "CONTRIBUTING.md" type thing up yet, but will now do so 👍

@kenotron
Copy link
Member

Would you terribly mind to make a PR with this?

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

No branches or pull requests

2 participants