Skip to content

Instantly share code, notes, and snippets.

@JonathonRichardson
Created December 17, 2022 14:03
Show Gist options
  • Save JonathonRichardson/d5963d5acbc96a50dba6905b90c84487 to your computer and use it in GitHub Desktop.
Save JonathonRichardson/d5963d5acbc96a50dba6905b90c84487 to your computer and use it in GitHub Desktop.
My Base ESLint Configuration
// This const doesn't actually do anything with regards to .eslintrc.js except
// document my preferred list of plugins, and allow me to document why I might
// choose any given plugin. You still have to manually install all of these,
// and they are not picked up because they're defined here.
const pluginsAndPackages = [
"@typescript-eslint/eslint-plugin",
"@typescript-eslint/parser",
"eslint-config-prettier",
"eslint-import-resolver-typescript",
"eslint-import-resolver-webpack",
"eslint-plugin-import",
"eslint-plugin-react",
"eslint-plugin-react-hooks",
"eslint-webpack-plugin",
];
// -----------------------
// | Warnings vs. Errors |
// -----------------------
//
// Before we begin, a note on warnings vs errors:
//
// Warnings are intended as an indication that something is probably okay for now, but should
// get fixed in the long run: it may or may not be a problem. In the case of coding, this means
// that they are things that are okay for during development, but that should be fixed before
// merging upstream.
//
// Errors are flat out problems that should block the compilation and execution of the code, because
// they are severe enough to keep the code from working. These should be limited to severe cases.
//
// You'll notice that in the configuration below, many rules are downgraded from "err" to "warn" based
// on this philosophy. For instance, `() => {}` may be a problem in production code, but it's common
// to create placeholders like that and `interface IProps {}` during rapid development. Those artifacts
// shouldn't make it to production, as they're code smelly, but they shouldn't overly alarm or block the
// developer to make them deal with them until the code is ready for Pull Requests and automated gateway
// checking (e.g. eslint, prettier, Jest testing, etc.)
//
// You should configure development time tools (e.g. webpack-dev-server) to tell you about warnings
// but not block on them and final build scripts (e.g. CI/CD pipelines) to block on both warnings and
// errors.
//
//
// ----------------------
// | Adopting New Rules |
// ----------------------
//
// There may come a time when you want to adopt a new rule. Here are some things to consider:
//
// DON'T adopt rules without autofixes. This is incredibly annoying to developers to be told that
// something trivial (e.g. var vs const from prefer-const) is flagged as an error or wanring and
// they have no idea how to fix them. If there isn't a good autofix (and sometimes even when there
// is), make sure to document potential fixes here in this documentation, as well as what you're
// trying to prevent by implementing this rule.
module.exports = {
// Define this as the root .eslint.js configuration. Individual libraries and services in
// the repo may have their own overrides, but this sets a nice baseline. See the ESLint
// documentation for more info:
//
// https://eslint.org/docs/latest/user-guide/configuring/configuration-files#cascading-and-hierarchy
root: true,
parser: "@typescript-eslint/parser",
plugins: ["@typescript-eslint", "import"],
extends: [
"eslint:recommended",
"plugin:react/recommended",
"plugin:react-hooks/recommended",
"plugin:import/recommended",
"plugin:import/typescript",
"prettier",
],
rules: {
// I don't do any configuration for JS files by default, because I don't ever write JS files
// by default. At this point, I write everything in Typescript and compile it down. If I
// have JavaScript files, it's almost always because they were legacy files written by either
// someone who wasn't me or someone who was me prior to ~2016, and there's no point in linting
// those. Those will likely just be ignored.
//
// On the off chance they're not ignored, it'd be just as annoying and time effective to convert
// the files to TypeScript as it would be to lint them, IMO.
//
// The only exception to this rule is for config files like this, but they are pretty simple
// and don't require massive introspection from linters.
},
// Only override configuration for ts files: https://stackoverflow.com/a/64488474/4339894
overrides: [
{
files: ["*.ts", "*.tsx"], // Your TypeScript file extensions
// As mentioned in the comments, you should extend TypeScript plugins here,
// instead of extending them outside the `overrides`.
// If you don't want to extend any rules, you don't need an `extends` attribute.
extends: ["plugin:@typescript-eslint/recommended", "plugin:@typescript-eslint/recommended-requiring-type-checking"],
// To make things easy to find, please keep these keys in alphabetical order.
rules: {
// Empty Functions are usually an indication that a developer left in some
// temporary code, but they can have serious performance problems since react
// can't tell that one instance of `() => {}` is the same as another.
//
// A common time people use empty lambdas/functions is in React component properties,
// especially in ternary operations:
// ```
// onClick={isValid ? submitHandler : () => {}}
// ```
// However, most React components accept `undefined` for event handlers, so you should
// just pass undefined instead of `() => {}`.
//
// If you truly need to pass a blank function, use a function from $Template/utils/empty-fn:
// - FUNCTION_THAT_DOES_NOTHING
// - FUNCTION_THAT_SHOULD_NOT_EVER_GET_CALLED
//
// If neither of these use cases match your current needs, add a new function to that
// module to indicate why this function is okay to be blank.
"@typescript-eslint/no-empty-function": ["warn"],
// I've considered disabling this entirely, but choosen against it in the long run.
//
// The reason for this rule is subtle and often seemingly a nuisance. We quite
// often include empty interfaces in Typescript to leave placeholders, especially
// for generics, and it can seem at first that there is no danger with empty interfaces,
// however, in Typescript, all empty interfaces are assignable to each other. This
// may not seem like a big deal, but it means that unrelated interfaces can be assignabled
// to each other and you get weird results from type guards, especially from options
// objects that can be valid as an empty object with no properties. I have witnessed
// a handful of bugs that while they were incredibly rare, took a disproportionately
// long time to debug and took a senior level developer to realize what was going on.
//
// At the hope of heading these off at the pass, we shouldn't use empty interfaces except
// as placeholders for later development. If we are attempting to use them as markers
// or something like that, including a single symbol property will serve that purpose
// without creating issues:
//
// ```
// const COLUMN_CONFIG_INTERFACE_INDICATOR = Symbol("Column Config");
//
// interface IColumnConfig {
// [COLUMN_CONFIG_INTERFACE_INDICATOR]: true;
// }
// ```
//
// You can then use a `createColumnConfig` utility function to avoid having to add so
// much boilerplate code:
//
// ```
// const createColumnConfig: (config: Exclude<IColumnConfig, typeof COLUMN_CONFIG_INTERFACE_INDICATOR>): IColumnConfig = (config) => {[COLUMN_CONFIG_INTERFACE_INDICATOR]: true, ...config};
// ```
//
// Additionally, since empty interfaces are common during development, we don't want to
// block compilation, so I'm marking this as warn, as they are fine during development
// but shouldn't be included in PRs.
//
// The only exception to this rule is props interfaces for components, as it makes it easier
// to augment props of components, especially in parallel, if there is already something
// in place for it, and this is already in the component template.
"@typescript-eslint/no-empty-interface": ["warn"],
"@typescript-eslint/no-unused-vars": [
// Declaring variables without using them is commonplace during development,
// so we'll only warn so we don't block the build, but these shouldn't happen
// in a PR. If you need to include something intentially that will be ignored,
// see the comments below for patterns.
"warn",
{
// If you want to include a variable on purpose (perhaps for inspection or
// debugging, or even future use), included "ignored", "debugValue", or
// "placeholder" in the name so that the compiler and any code reviewers
// know why it's there.
varsIgnorePattern: "([iI]gnored)|([dD]ebugValue$)|([pP]laceHolder)",
// The default of this is "false", but it's important to ignore these,
// because rest siblings (e.g. `{ paramToPullOut, ...props }`) can be
// used to extract out properties from an object that we don't want to
// pass into a third party component/options object/etc, and there's no
// good way to indicate this otherwise (like including "ignore") in the
// variable name.
ignoreRestSiblings: true,
// Allow us to include argument references to variables we plan to ignore
// for now by prefixing them with an underscore. Also allow "props" to be
// used for component functions without warnings, since it's common to have
// that for convenience for later augmentation. Same with "e" for event
// handlers
argsIgnorePattern: "(^_)|(^props$)|(^e$)",
// FYI, there was a bug where this was not recognized which was fixed in the
// 5.17 version of @typescript-eslint/eslint-plugin (Released on 2022-03-28)
// - https://github.com/typescript-eslint/typescript-eslint/issues/4691
// - https://github.com/typescript-eslint/typescript-eslint/pull/4748
destructuredArrayIgnorePattern: "^_",
},
],
// This just mainly helps prevent merge conflicts and keeps the code clean.
"import/order": ["warn"],
// Turn off the "no-unused-vars" rule because we'll be using the @typescript-eslint/no-unused-vars
// rule instead.
// - https://typescript-eslint.io/rules/no-unused-vars/#how-to-use
"no-unused-vars": "off",
// There's no reason to block on this. It's common to have `let`s around while writing code
// before you get to writing the code that might modify it.
"prefer-const": ["warn"],
// "jsx-sort-props", unlike sort-keys, does warrant being here to prevent merge conflicts.
"react/jsx-sort-props": [
"warn",
{
ignoreCase: true,
},
],
// If you're doing this, you're almost certainly doing something wrong. There are a couple edge
// cases regarding integrations with third party non-react libraries and writing advanced render
// interactions with the DOM where this may be necessary, but it usually just causes code
// injection attack surfaces. Do NOT override this locally unless you really really really
// know what you're doing, and even then, you probably shouldn't.
"react/no-danger": ["error"],
// Don't forbid ' and " in literals, because they make the code way easier to read.
"react/no-unescaped-entities": ["error", { forbid: [">", "}"] }],
// Typescript does a good enough job of checking props. There is good reason to use prop-types
// when you're also using TypeScript typed props (unless perhaps you needed to integrate with some
// other library that needs runtime checks on props, but you can just override this in that project
// locally, then).
"react/prop-types": ["off"],
// exhaustive-deps is often temporarily violated while first writing hooks, so this is
// downgraded to a "warn" status.
"react-hooks/exhaustive-deps": "warn",
// Hooks are wonderful, but, if not used properly, are an avenue for so many hard to
// trace bugs to seep into your code. Fortunately, the React team publishes these
// rules.
//
// Violating the rules of hooks is generally very bad and can cause such bad side effects
// that it is considered an error.
"react-hooks/rules-of-hooks": "error",
//"sort-imports": ["warn"],
// Disabling sort-keys. Although it would help alleviate a few issues regarding
// merge conflicts, in practice these are fairly few and far between. In addition,
// it's common to want to group "like" keys near each other (albeit you could argue
// those "like" keys should be on a child object), and it's nicer to include keys in
// the order they appear in interface declarations or documentation, which aren't
// themselves easily orderable.
//
// The final nail in the coffin for using this rule is that there is no auto-fix,
// so developer have to waste time and mental cycles manually sorting keys, which
// is more of a waste of time than dealing with the merge conflicts. I'd still
// reccommend going alphabetical whenever possible, but enforcing it as a rule
// is unnecessary.
//
// For reference, if you do want to enable this, I used to use this config:
//
// "sort-keys": [
// "warn",
// "asc",
// {
// caseSensitive: false,
// natural: true,
// },
// ],
//
// Also, it should be noted that the order of keys in JavaScript does actually matter
// and have significance, albeit really nuanced edge cases, like the order to return
// listing keys, etc, so there are rare cases, especially with external libraries that
// naïvely assume their keys will be in a certain order: which is given as per the spec
// but a really fragile assumption full of code smell.
"sort-keys": ["off"],
},
parserOptions: {
tsconfigRootDir: __dirname,
project: ["./tsconfig.json"], // Specify it only for TypeScript files
},
},
],
settings: {
react: {
// React version. "detect" automatically picks the version you have installed.
//
// You can also use `16.0`, `16.3`, etc, if you want to override the detected value.
version: "detect",
},
"import/resolver": "webpack",
},
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment