eslint default rules implementation

[brianpmccullough]

Hello, I am digging in recently on the ESLint configuration in SPFx projects. I am looking at SPFx 1.20, rather than earlier versions.

I am thinking about organizations who are accumulating lots of SPFx repositories and exposed to configuration "drift". That is, inconsistently applied eslint configurations between various projects. With all the rules copied into the eslintrc.js, it's difficult to tell, quickly, which rules are overridden from the SPFx defaults.

I notice that in my React based applications, I get, at a high level:

1. A configuration that extends from "@microsoft/eslint-config-spfx/lib/profiles/react", which inturn extends @microsoft/eslint-config-spfx/lib/profiles/default
2. LOTS of rules defined granularly
3. A separate overrides section for unit test related files, with NO rule overrides specified by default.

I am aware of another eslint discussion started by @andrewconnell a while back (see: #8290). Admittedly, at the time, this conversation was lost on me, but I think I understand ESLint configuration a bit more now.

If we have the ability to override the rules in our projects, why does SPFx not provide a base set of rules in their configuration, and instead copy/paste the rules into the eslintrc.js in EVERY project (possibly after @AJIXuMuK response to @andrewconnell here: #8290 (comment))? Or, perhaps, just add an additional item in extends like "@microsoft/eslint-config-spfx/lib/profiles/default-rules", which if you prefer to not have opinionated defaults (like Andrew), you could simply remove the "default-rules" from the "extends"? The latter might have the additional benefit of versioning the default rules with each SPFx release, allowing teams during upgrade to choose to delay moving to a new set of rules, if for example, it causes a bunch of new errors/warnings.

I notice in SPFx 1.20 now, there are two configurations in microsoft/eslint-config-spfx/lib/profiles/default, so it appears SPFx team are not opposed to embedding some default behaviors in the default eslint config.

    overrides: [
        {
            files: ['*.ts', '*.tsx'],
            rules: {
                '@typescript-eslint/no-require-imports': 'off',
                '@typescript-eslint/no-empty-object-type': 'off'
            }
        }
    ]

In lieu of Microsoft NOT providing their default rule configurations centrally, what if a central package was created that maintained the SPFx defaults. Could perhaps be a Patterns and Practices (PnP) package?

This way, a project's eslintrc.js might look instead like so:

require('@rushstack/eslint-config/patch/modern-module-resolution');
module.exports = {
  extends: ['@microsoft/eslint-config-spfx/lib/profiles/react', 
                  //*******************************************************************************
                  //NOTE: react-defaults doesn't actually exist, I am proposing maybe it should?
                  //*******************************************************************************
                  '@microsoft/eslint-config-spfx/lib/profiles/react-defaults'
                  ],
  parserOptions: { tsconfigRootDir: __dirname },
  overrides: [
    {
      files: ['*.ts', '*.tsx'],
      rules: {
        //*******************************************************************************
        //NOW I ONLY NEED TO OVERRIDE WHAT I WANT DIFFERENT FROM DEFAULT           
       //*******************************************************************************
      }
    },
    {
        //*******************************************************************************
        // TEST FILES SECTION TRIMMED FOR BREVITY           
       //*******************************************************************************
      // For unit tests, we can be a little bit less strict.  The settings below revise the
      // defaults specified in the extended configurations, as well as above.
      files: [],
      rules: {}
    }
  ]
};

For reference, an SPFx 1.20 OOB React eslintrc.js:

require('@rushstack/eslint-config/patch/modern-module-resolution');
module.exports = {
  extends: ['@microsoft/eslint-config-spfx/lib/profiles/react'],
  parserOptions: { tsconfigRootDir: __dirname },
  overrides: [
    {
      files: ['*.ts', '*.tsx'],
      parser: '@typescript-eslint/parser',
      'parserOptions': {
        'project': './tsconfig.json',
        'ecmaVersion': 2018,
        'sourceType': 'module'
      },
      rules: {
        //*******************************************************************************
        //LOTS AND LOTS OF RULES GRANULARLY DEFINED
        //*******************************************************************************
      }
    },
    {
      // For unit tests, we can be a little bit less strict.  The settings below revise the
      // defaults specified in the extended configurations, as well as above.
      files: [
        // Test files
        '*.test.ts',
        '*.test.tsx',
        '*.spec.ts',
        '*.spec.tsx',

        // Facebook convention
        '**/__mocks__/*.ts',
        '**/__mocks__/*.tsx',
        '**/__tests__/*.ts',
        '**/__tests__/*.tsx',

        // Microsoft convention
        '**/test/*.ts',
        '**/test/*.tsx'
      ],
      rules: {}
    }
  ]
};

[brianpmccullough]

I took a stab at what this might look like: https://github.com/brianpmccullough/spfx-eslint