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

ESLint import/order rule should not enforce grouping of imports #57368

Open
snickell opened this issue Mar 19, 2024 · 1 comment
Open

ESLint import/order rule should not enforce grouping of imports #57368

snickell opened this issue Mar 19, 2024 · 1 comment
Assignees

Comments

@snickell
Copy link
Contributor

snickell commented Mar 19, 2024

I'm seeing some behavior I don't like with the import/order rule around it trying to enforce single-groups, rather than trusting the human to create import groups that make semantic sense to them. I think this is more important than alphabetization, and personally, if I had to chose one, I'd rather have the ability for humans to group imports (and encourage people to do this) than have them in one large block in alphabetical order.

I tried applying eslint to webpack.config.js, and it went from this:

const webpack = require('webpack');
const path = require('path');
const sass = require('sass');

// Webpack Plugins:
const {BundleAnalyzerPlugin} = require('webpack-bundle-analyzer');
const CopyPlugin = require('copy-webpack-plugin');
const LiveReloadPlugin = require('webpack-livereload-plugin');
const {StatsWriterPlugin} = require('webpack-stats-plugin');
const TerserPlugin = require('terser-webpack-plugin');
const UnminifiedWebpackPlugin = require('unminified-webpack-plugin');
const {WebpackManifestPlugin} = require('webpack-manifest-plugin');
const WebpackNotifierPlugin = require('webpack-notifier');
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
const {PyodidePlugin} = require('@pyodide/webpack-plugin');
const CircularDependencyPlugin = require('circular-dependency-plugin');

To this (which it still errored out on due to the newline, but if it had succeeded, it would not have allowed webpack plugins to be a separate group o fimports from path, sass, and webpack, which I think lowers code quality):

const {PyodidePlugin} = require('@pyodide/webpack-plugin');
const CircularDependencyPlugin = require('circular-dependency-plugin');
const CopyPlugin = require('copy-webpack-plugin');
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
const path = require('path');
const sass = require('sass');
const TerserPlugin = require('terser-webpack-plugin');
const UnminifiedWebpackPlugin = require('unminified-webpack-plugin');
const webpack = require('webpack');

// Webpack Plugins:
const {BundleAnalyzerPlugin} = require('webpack-bundle-analyzer');
const LiveReloadPlugin = require('webpack-livereload-plugin');
const {WebpackManifestPlugin} = require('webpack-manifest-plugin');
const WebpackNotifierPlugin = require('webpack-notifier');
const {StatsWriterPlugin} = require('webpack-stats-plugin');

Firstly, related to my comment about comments, it looks like the answer is that import/order as we've configured it can't handle comments, which is not great. Secondly, I believe that grouping imports with comments is a very good pattern, and the is a human judgement call that shouldn't be enforced by the linter.

My ideal here would be if we could configure the linter to treat a newline as a group, and to enforce alphabetical within the group, but not enforce grouping everything into a huge block of imports.

@snickell
Copy link
Contributor Author

What I would like to see the plugin do here is sort this block:

const path = require('path');
const sass = require('sass');
const webpack = require('webpack');

And then this block:

// Webpack Plugins:
const CircularDependencyPlugin = require('circular-dependency-plugin');
const CopyPlugin = require('copy-webpack-plugin');
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
const TerserPlugin = require('terser-webpack-plugin');
const UnminifiedWebpackPlugin = require('unminified-webpack-plugin');
const {BundleAnalyzerPlugin} = require('webpack-bundle-analyzer');
const LiveReloadPlugin = require('webpack-livereload-plugin');
const {WebpackManifestPlugin} = require('webpack-manifest-plugin');
const WebpackNotifierPlugin = require('webpack-notifier');
const {PyodidePlugin} = require('@pyodide/webpack-plugin');
const {StatsWriterPlugin} = require('webpack-stats-plugin');

But let the human decide how to decompose imports into groups, with each group delineated by a newline between.

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