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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESLint: Add and configure import plugin #219

Merged
merged 1 commit into from
Sep 1, 2020
Merged

ESLint: Add and configure import plugin #219

merged 1 commit into from
Sep 1, 2020

Conversation

tfrommen
Copy link
Contributor

This is a first pass for #84, making use of the already available ESLint import plugin (and not the sort-imports rule).

I decided to only pull in rules that are considered errors:

As indicated, I disabled the last rule, no-unresolved, as we make use of externals, mostly for @wordpress/** packages and things such as react or lodash.
Another way to go about this would be to add exceptions to the no-unresolved rule. This is easily possible, but since this might differ, both between various projects, and over time, in general, I thought it best to disable therule.

I also manually added and configured the import/order rule, and, unless I madea mistake, here's how it should expect import statements to occur:

  1. Node.JS built-in modules such as path.
  2. Regular external packages specified in the package.json file.
  3. @wordpress/** packages.
  4. Local files in a parent folder,
  5. Local sibling files, meaning in the current file's folder.
  6. A potential main index.(js|ts)x? file.

Each block has to be ordered alphabetically, case-insensitive, with regard to the package name and file path, respectively. And there has to be single blank line between any two subsequent blocks from the above list.

I also added two fixture files:

  1. pass:
/* eslint-disable no-unused-vars */

import path from 'path';

import chalk from 'chalk';
import eslint from 'eslint';

import apiFetch from '@wordpress/api-fetch';

import index from '../../index';
import '../test-lint-config';

import Test from './component-jsx-parentheses';

import './';
import './style.scss';
  1. fail:
/* eslint-disable no-unused-vars */

import apiFetch from '@wordpress/api-fetch';
import chalk from 'chalk';
import eslint from 'eslint';
import path from 'path';
import './';
import Test from './component-jsx-parentheses';
import './style.scss';
import index from '../../index';
import '../test-lint-config';

Please provide some feedback, and maybe even try this out. Thanks! 馃檹

Copy link
Contributor

@mikeselander mikeselander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me 馃憤

@tfrommen
Copy link
Contributor Author

tfrommen commented Sep 1, 2020

@rmccue @kadamwhite @ntwb @igmoweb anyone have any objections? Or are we good to merge this? 馃檪

@kadamwhite
Copy link
Contributor

@tfrommen Will this sort itself out if you run eslint --fix?

@tfrommen
Copy link
Contributor Author

tfrommen commented Sep 1, 2020

@kadamwhite yes.

Copy link
Contributor

@kadamwhite kadamwhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@kadamwhite kadamwhite merged commit e01865a into master Sep 1, 2020
@tfrommen tfrommen deleted the import branch September 1, 2020 22:23
@igmoweb
Copy link
Contributor

igmoweb commented Sep 2, 2020

I'm late but I like it 馃憤

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

Successfully merging this pull request may close these issues.

None yet

4 participants