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

Option to enable fork-ts-checker-webpack-plugin's ESLint support #7936

Closed
martpie opened this issue Jul 12, 2019 · 7 comments
Closed

Option to enable fork-ts-checker-webpack-plugin's ESLint support #7936

martpie opened this issue Jul 12, 2019 · 7 comments

Comments

@martpie
Copy link
Contributor

martpie commented Jul 12, 2019

Feature request

Is your feature request related to a problem? Please describe.

fork-ts-checker-webpack-plugin just shipped an option to enable linting with ESLint, this could allow devs to see TypeScript errors as well as ESLint errors in the console in dev mode. It could also be used to fail on build.

I agree this is a bit unconventional as Next.js is not really tight to ESLint, still it would be neat.

I could help implementing it if needed.

Describe the solution you'd like

Have an options in next.config.js to enable fork-ts-checker-webpack-plugin eslint option.

Describe alternatives you've considered

Call fork-ts-checker-webpack-plugin manually by modifying the Webpack config

Additional context

n/a

@kachkaev
Copy link
Contributor

It would be great if this feature was considered together with #7687, which is getting some popularity despite being closed.

@fabb
Copy link
Contributor

fabb commented Aug 2, 2019

I tried updating the options of the fork-ts-checker-webpack-plugin that is configured by next out of the box, but I could not make it work:

  1. Just update the options of the existing plugin instance
const existingForkTsCheckerWebpackPlugin = config.plugins.find(plugin => plugin.constructor.name === 'ForkTsCheckerWebpackPlugin')
if (typeof existingForkTsCheckerWebpackPluginIndex !== 'undefined') {
    existingForkTsCheckerWebpackPlugin.options.eslint = true
}

⛔️ Does not work because the fork-ts-checker-webpack-plugin does not allow to update options after construction, because in its constructor, it configures some stuff based on the options.

  1. Read out the options of the existing plugin instance, and replace the instance with a new one with updated options
const existingForkTsCheckerWebpackPluginIndex = config.plugins.findIndex(plugin => plugin.constructor.name === 'ForkTsCheckerWebpackPlugin')
if (existingForkTsCheckerWebpackPluginIndex !== -1) {
    const existingForkTsCheckerWebpackPlugin = config.plugins[existingForkTsCheckerWebpackPluginIndex]
    const originalOptions = existingForkTsCheckerWebpackPlugin.options
    const updatedOptions = { ...originalOptions, eslint: true }
    const replacementForkTsCheckerWebpackPlugin = new (require('fork-ts-checker-webpack-plugin'))(updatedOptions)
    config.plugins[existingForkTsCheckerWebpackPluginIndex] = replacementForkTsCheckerWebpackPlugin
}

⛔️ Does not work because it breaks the type error console log output which makes it useless. I am not sure why this is, but I guess it it has to do something with next setting compiler hooks on fork-ts-checker-webpack-plugin for logging, and replacing the instance somehow breaks this compiler hook.

If I set silent: false in the options, at least I would see the original log output from the plugin, but the solution is not really satisfactory as long as I don't understand why the compiler hook breaks.

@Timer
Copy link
Member

Timer commented Dec 23, 2019

We're not going to expose this variable as the fact that we use this package is an implementation detail.

If you'd like to turn on ESLint support, please customize the webpack configuration (which isn't covered by semver).

@Timer Timer closed this as completed Dec 23, 2019
@martpie
Copy link
Contributor Author

martpie commented Dec 25, 2019

Fair point, thanks 👍

@Timer
Copy link
Member

Timer commented Dec 25, 2019

FYI we are planning an ESLint integration soon!

@neo
Copy link

neo commented May 21, 2020

@Timer do you mind sharing some update about this? 😳

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants