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

Flatten eslint configs, removing config/rules #338

Merged
merged 2 commits into from
Jun 28, 2022
Merged

Conversation

BPScott
Copy link
Member

@BPScott BPScott commented Jun 27, 2022

Description

The config definitions in lib/configs, import contents from lib/configs/rules. This extra layer of indirection adds no value - nothing is ever reused.

This PR flattens this indirection, replacing require calls in lib/configs/*.js with the contents of the files being required.

This has no affect on the rulesets used, it's just tidying.

To test

Compare the evaluated output of the lib/config/*.js files on main, and in this branch and note that they are identical.

Copy the following script into the root of the repo as test.js:

const glob = require('glob');

const fileEval = glob
  .sync('./packages/eslint-plugin/lib/config/*.js')
  .map((filename) => {
    return [filename, JSON.stringify(require(filename), validatingReplacer, 2)];
  });

const formattedResult = fileEval
  .map(
    ([filename, evaluatedContent]) =>
      `## Filename: ${filename}\n\n\`\`\`\n${evaluatedContent}\n\`\`\``,
  )
  .join('\n\n');

console.log(formattedResult);

//
// UTILS
//

function validatingReplacer(key, value) {
  if (
    ['number', 'string', 'boolean'].includes(typeof value) ||
    Array.isArray(value) ||
    isPlainObject(value) ||
    value === null
  ) {
    return value;
  }

  console.log({key, value});
  throw new Error(`Found an unknown value for key ${key}!`);
}

function isPlainObject(value) {
  if (typeof value !== 'object' || value === null) {
    return false;
  }

  const prototype = Object.getPrototypeOf(value);
  return (
    (prototype === null ||
      prototype === Object.prototype ||
      Object.getPrototypeOf(prototype) === null) &&
    !(Symbol.toStringTag in value) &&
    !(Symbol.iterator in value)
  );
}
  • Run the script on the main branch, run the script and save the output to a file node test.js > main-config-content.md
  • Check out this branch, run the script and save the output to a file: node test.js > branch-config-content.md
  • Diff the output of those two files and note that they are identical diff -u main-config-content.md branch-config-content.md

Note that modifying some file in lib/config and doing the above again shall result in some diff output to prove that changes are noticed.

This extra layer of requires adds no value. Pull rule configs into the
lib/config/* files
@BPScott BPScott requested a review from a team June 27, 2022 22:57

// Augmenting configs - When extending, these go after the core config
graphql: require('./lib/config/graphql'),
jest: require('./lib/config/jest'),
node: require('./lib/config/node'),
polaris: require('./lib/config/polaris'),
react: require('./lib/config/react'),
Copy link
Member Author

Choose a reason for hiding this comment

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

React and typescript-type-checking are both augment configs, not core configs, per the README.

Copy link
Contributor

@vsumner vsumner left a comment

Choose a reason for hiding this comment

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

Much better.

@BPScott BPScott merged commit d8c61cc into main Jun 28, 2022
@BPScott BPScott deleted the flatten-eslint-configs branch June 28, 2022 16:43
@github-actions github-actions bot mentioned this pull request Jun 28, 2022
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

2 participants