wotan icon indicating copy to clipboard operation
wotan copied to clipboard

A plugin for Webpack

Open ljani opened this issue 7 years ago • 8 comments

I'd like to know when an official plugin is released for this. So, maybe provide an update in this issue when there is one?

Here's a very rudimentary plugin for Webpack 4 based on your LintCommandRunner:

import { Container, BindingScopeEnum, injectable } from "inversify";
import { Compiler, Plugin } from "webpack";
import {
    MessageHandler,
    Failure
} from "@fimbul/ymir";
import {
    CachedFileSystem,
    createCoreModule,
    createDefaultModule,
    FormatterLoader,
    LintOptions,
    Runner
} from "@fimbul/wotan";

export interface LintCommand extends LintOptions {
    formatter: string | undefined;
}

// https://github.com/fimbullinter/wotan/blob/be4483d5885c88a9640869203e46aba58d49e8f7/packages/wotan/src/commands/lint.ts
@injectable()
class LintCommandRunner {
    constructor(
        private runner: Runner,
        private formatterLoader: FormatterLoader,
        private logger: MessageHandler,
        private fs: CachedFileSystem,
    ) {
    }
    public run(options: LintCommand) {
        const formatter = new (this.formatterLoader.loadFormatter(options.formatter === undefined ? 'stylish' : options.formatter))();
        const result = this.runner.lintCollection(options);
        let success = true;
        if (formatter.prefix !== undefined)
            this.logger.log(formatter.prefix);
        for (const [file, summary] of result) {
            if (summary.failures.some(isError))
                success = false;
            const formatted = formatter.format(file, summary);
            if (formatted !== undefined)
                this.logger.log(formatted);
            if (options.fix && summary.fixes)
                this.fs.writeFile(file, summary.content);
        }
        if (formatter.flush !== undefined) {
            const formatted = formatter.flush();
            if (formatted !== undefined)
                this.logger.log(formatted);
        }
        return success;
    }
}

function isError(failure: Failure) {
    return failure.severity === 'error';
}

export default class WotanWebpackPlugin implements Plugin {
    public constructor(private options: LintCommand) {
    }

    public afterCompile(): void {
        const container = new Container({defaultScope: BindingScopeEnum.Request});
        container.load(createCoreModule({}), createDefaultModule());
        container.bind<LintCommandRunner>(LintCommandRunner)
            .toSelf();
        container.get(LintCommandRunner)
            .run(this.options);
    }

    public apply(compiler: Compiler): void {
        compiler.hooks
            .done
            .tap("WotanWebpackPlugin", this.afterCompile.bind(this));
    }
}

Usage:

new WotanWebpackPlugin({
    config: resolve(paths.tools, "wotan.yaml"),
    project: resolve(paths.src, "tsconfig.json"),
    files: [],
    exclude: [],
    fix: false,
    extensions: undefined,
    formatter: undefined
})

ljani avatar Mar 22 '18 13:03 ljani

Thank you for the feature request and this example code. I think a webpack plugin would be an awesome addition.

I don't have any experience with writing webpack plugins. So here are some random thoughts after having a first look at it:

  • instead of reading all files from disk again (that's what the default NodeFileSystem does), there should be a way to use the already loaded files from the current webpack compilation
  • it should read and parse .fimbullinter.yaml file like the CLI to load plugin modules and use it for default values
  • is there a way to add a watch for files loaded by the linter (tsconfig.json, .wotanrc.yaml, .fimbullinter.yaml, ...) so that a new linting run is triggered if the config changes? -- I looked it up, you can
  • is it allowed (or even encouraged) to write the fixed files directly to disk?
  • is there a way to use the same project settings as the typescript-loader used to load the sources?

I'll look into exposing some utilities so that there's no need to duplicate all the code.

ajafff avatar Mar 22 '18 14:03 ajafff

The plugin hooks should not throw exceptions, otherwise the process seems to hang.

Here's an improved version:

export default class WotanWebpackPlugin implements Plugin {
    public constructor(private options: LintCommand) {
    }

    public afterCompile(compilation: webpack.compilation.Compilation): void {
        try {
            this.run();
        } catch (e) {
            compilation.errors.push(e);
        }
    }

    private run(): void {
        const container = new Container({defaultScope: BindingScopeEnum.Request});
        container.load(createValtyrModule(), createCoreModule({}), createDefaultModule());
        container.bind<LintCommandRunner>(LintCommandRunner)
            .toSelf();
        container.get(LintCommandRunner)
            .run(this.options);
    }

    public apply(compiler: Compiler): void {
        compiler.hooks
            .afterCompile
            .tap("WotanWebpackPlugin", this.afterCompile.bind(this));
    }
}

However the plugin does not seem to work at the moment. I get the following error after updating from v0.6.0 even if createDefaultModule() is still there:

Error: No matching bindings found for serviceIdentifier: MessageHandler

EDIT: it might be related to importing reflect-metadata, see InversifyJS. EDIT2: v0.7.0 is okay and v0.8.0 fails. EDIT3: Ah, there are multiple versions of eg. ymir, because of #258. I'd guess fixing those would help this as well. EDIT4: With v0.10.0 this is working again. EDIT5: You might prefer .done after all and use a different way of logging errors, because .afterCompile will run twice for some reason.

ljani avatar May 18 '18 10:05 ljani

Hi, guys. First of all thanks for all your work. I stumbled with wotan because I was using TS with vue. Anyway. I'm kind of noob with webpack. Could you please explain how would I implement this plugin to use wotan with webpack?

Camotubi avatar Jun 12 '18 00:06 Camotubi

Basically you'd save the class WotanWebpackPlugin and related stuff to a file, import it in your configuration file and add the new WotanWebpackPlugin(...) to the plugins section of your Webpack configuration file and it should run after webpack has built the stuff. The lifecycle is still a bit of a question mark for me as well, and thus you might investigate other options than .done or .afterCompile to hook into Webpack's lifecycle to gain better performance.

ljani avatar Jun 12 '18 13:06 ljani

Here's another take where the linter is forked into background:

import {autobind} from "core-decorators";
import {ChildProcess, spawn} from "child_process";
import {EOL} from "os";
import {compilation, Compiler, Plugin} from "webpack";
import webpackLog from "webpack-log";

export interface IOptions {
    config: string;
    project: string;
    fix?: boolean;
}

const pluginName = "WotanWebpackPlugin";

function promiseFromChildProcess(child: ChildProcess): Promise<number> {
    return new Promise(function (resolve, reject) {
        function error() {
            cleanup();
            reject();
        }

        function exit(status: number) {
            cleanup();
            resolve(status);
        }

        function cleanup() {
            child.removeListener("error", error);
            child.removeListener("exit", exit);
        }

        child.addListener("error", error)
            .addListener("exit", exit);
    });
}

@autobind
export default class WotanWebpackPlugin implements Plugin {
    private childProcess: ChildProcess | null = null;
    private watch: boolean = false;
    private log = webpackLog({
        name: pluginName
    });

    public constructor(private options: IOptions) {
    }

    public apply(compiler: Compiler): void {
        const {
            startWatch,
            runWotan
        } = this;

        const {
            watchRun,
            afterEmit
        } = compiler.hooks;

        const name = pluginName;
        watchRun.tap(name, startWatch);
        afterEmit.tapPromise(name, runWotan);
    }

    private startWatch(): void {
        this.watch = true;

        const {
            childProcess,
            log
        } = this;

        this.childProcess = null;
        if (childProcess) {
            log.info("Restarting the background process");
            childProcess.kill();
        }
    }

    private async runWotan(compilation: compilation.Compilation): Promise<void> {
        const {
            options: {
                config,
                project,
                fix
            },
            childProcess,
            childProcessError,
            childProcessExit,
            log,
            watch
        } = this;

        if (childProcess) {
            const message = "Did not expect a childProcess at this point";
            log.error(message);
            throw new Error(pluginName + ": " + message);
        }


        let args = [
            "--module",
            "@fimbul/valtyr",
            "--config",
            config,
            "--project",
            project
        ];

        if (fix) {
            args = args.concat("--fix");
        }

        const command = "wotan";
        const options = {
            shell: true,
        };

        let stdio;
        if (watch) {
            stdio = ["ignore", "inherit", "inherit"];
            log.info("Running the background process");
        } else {
            stdio = ["ignore", "pipe", "pipe"];
            log.info("Running linter");
        }

        const newChildProcess = spawn(command, args, {
            ...options,
            stdio
        })
            .on("exit", childProcessExit)
            .on("error", childProcessError);
        this.childProcess = newChildProcess;

        if (watch) {
            // Don't care what happens with the linter
            return;
        }

        let message = pluginName + EOL;
        const capture = (chunk: string) => message += chunk;
        const {
            stdout,
            stderr,
        } = newChildProcess;
        stdout.on("data", capture);
        stderr.on("data", capture);

        try {
            const status = await promiseFromChildProcess(newChildProcess);
            if (status === 0) {
                return;
            }

            message += EOL;
            message += pluginName;
            message += ": linter failed with code ";
            message += status;

            const error = new Error(message);
            compilation.errors
                .push(error);
        } catch (e) {
            throw e;
        } finally {
            stdout.off("data", capture);
            stderr.off("data", capture);
        }
    }

    private childProcessError(): void {
        const {
            childProcessExit,
            log
        } = this;

        log.error("Running the linter failed");
        childProcessExit();
    }


    private childProcessExit(): void {
        const {
            childProcess,
            childProcessError,
            childProcessExit
        } = this;
        this.childProcess = null;

        if (childProcess) {
            childProcess
                .off("exit", childProcessExit)
                .off("error", childProcessError);
        }
    }
}

ljani avatar Jul 02 '18 06:07 ljani

Another point to consider: create-react-app has gained support for Typescript, but they do not support tslint yet (only eslint for .js files). They are looking into adding support for linting .ts files. I wonder how one would use wotan in future instead of tslint with create-react-app without running eject.

ljani avatar Jan 09 '19 11:01 ljani

@ljani can you provide a link to the relevant discussion? I suppose, since this is in the issue about the webpack plugin, that this integration should be a webpack plugin?

ajafff avatar Jan 09 '19 12:01 ajafff

They're discussing tslint integration at https://github.com/facebook/create-react-app/issues/5641.

From what I can tell at the moment, yes, the integration should be a webpack plugin. Currently create-react-app is aiming for tslint support via fork-ts-checker-webpack-plugin, which has some form of tslint support.

If the language server plugin could report warnings/errors during build, that would be ideal, but I've understood it's not possible?

EDIT: To be clear, there is no support for adding arbitrary webpack plugins in create-react-app without eject. I don't know what would be the proper integration at the moment, but it's likely to be related to webpack.

ljani avatar Jan 09 '19 12:01 ljani