Skip to content

Conversation

johnnyreilly
Copy link
Contributor

Hello everybody!

I've a mini PR which you're welcome to not take, but it would make me very happy if you did 😄

I am an inveterate fiddler and tweaker. As such, I'm the sort of person who likes to either eject my create-react-app or use something like craco to customise it. I'm aware that this is on me and that there are no guarantees offered for customisation. Nevertheless, I've a tiny tweak to offer that I think might still be worth considering.

A common thing for me to do when tweaking is to use the fork-ts-checker-webpack-plugin to handle ESLint as well as type checking. There's definitely some ego in there; it was me that added this ability to the plugin. However, it's also that I appreciate using a consistent mechanism for reporting issues, be they type checking or lint issues.

Consider the following craco recipe:

/* eslint-disable @typescript-eslint/no-var-requires */
const ESLintPlugin = require('eslint-webpack-plugin');
const ForkTsCheckerWebpackPlugin = require('react-dev-utils/ForkTsCheckerWebpackPlugin');

const { throwUnexpectedConfigError } = require('@craco/craco');

const throwError = (message) =>
    throwUnexpectedConfigError({
        packageName: 'craco',
        githubRepo: 'gsoft-inc/craco',
        message,
        githubIssueQuery: 'webpack',
    });

module.exports = {
    webpack: {
        configure: (webpackConfig, { env, paths }) => {
            console.log('removing ESLintPlugin');
            const indexOfESLintPlugin = webpackConfig.plugins.findIndex((plugin) => plugin instanceof ESLintPlugin);
            if (indexOfESLintPlugin === -1) throwError('failed to find ESLintPlugin');
            const removed = webpackConfig.plugins.splice(indexOfESLintPlugin, 1);
            if (removed.length !== 1) throwError('failed to remove ESLintPlugin');
            console.log('removed ESLintPlugin', indexOfESLintPlugin);

            console.log('activate ESLint in fork-ts-checker-webpack-plugin');
            const indexOfForkTsCheckerWebpackPlugin = webpackConfig.plugins.findIndex(
                (plugin) => plugin instanceof ForkTsCheckerWebpackPlugin
            );
            if (indexOfForkTsCheckerWebpackPlugin === -1) throwError('failed to find ESLintPlugin');
            const forkTsCheckerWebpackPlugin = webpackConfig.plugins[indexOfForkTsCheckerWebpackPlugin];
            forkTsCheckerWebpackPlugin.eslint = true;
            forkTsCheckerWebpackPlugin.eslintOptions = {
                cache: false,
                eslintPath: require.resolve('eslint'),
                resolvePluginsRelativeTo: __dirname,
            };
            console.log('activated ESLint in fork-ts-checker-webpack-plugin');

            return webpackConfig;
        },
    },
};

This removes the eslint-webpack-plugin and turns on ESLint support in the fork-ts-checker-webpack-plugin. Running this config on a project with a simple linting issue in it surfaces up the following:

Please note, this works fine apart from one thing; the undefined error message. This is because the origin is "eslint" and that key does not exist in issueOrigins. Hence the undefined as issueOrigins[origin] fails to look up an entry.

If we add in an eslint entry like so:

const issueOrigins = {
  typescript: 'TypeScript',
  internal: 'fork-ts-checker-webpack-plugin',
   eslint: 'ESLint' // prevents 'undefined error' message if people have enabled ESLint via fork-ts-checker-webpack-plugin
};

Then this issue is resolved and we get a nice console message like so:

Of course, this is a niche issue, but I know I'm not the only person who would love this to be taken. So... what do you think?

@johnnyreilly
Copy link
Contributor Author

There's a multitude of build failures, but having looked at it, it seems like azure pipelines is acting up.

@stale
Copy link

stale bot commented Jan 9, 2022

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants