Skip to content

Replace eslint rulesdir with eslint-plugin-local, convert eslint rules to JS#50380

Merged
jakebailey merged 18 commits intomicrosoft:mainfrom
jakebailey:convert-eslint-rules
Aug 22, 2022
Merged

Replace eslint rulesdir with eslint-plugin-local, convert eslint rules to JS#50380
jakebailey merged 18 commits intomicrosoft:mainfrom
jakebailey:convert-eslint-rules

Conversation

@jakebailey
Copy link
Copy Markdown
Member

@jakebailey jakebailey commented Aug 20, 2022

--rulesdir is deprecated in favor of plugins. Although there's no official support for a local plugin, eslint-plugin-local can do what we need. This, combined with converting our rules to JS, means that we don't need any configuration in VS Code or to run the CLI, nor do we need a build step to run after clone or branch changes (leaving the diagnostics as the only remaining build step on clone/branch change).

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 20, 2022
@jakebailey
Copy link
Copy Markdown
Member Author

The only gotcha here is that people working on branches between these two configs may get an error in their editor, depending on which config they have in .vscode/settings.json, since we don't check that in.

const { createRule } = require("./utils");

export = createRule({
module.exports = createRule({
Copy link
Copy Markdown
Member Author

@jakebailey jakebailey Aug 22, 2022

Choose a reason for hiding this comment

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

These are all now CJS (well, they were before after compilation, but now we're hand-writing that); a future version of ESLint is poised to completely revamp the way that configuration works and will likely enable ESM rules.

const ext = ".js";
const ruleFiles = fs.readdirSync(rulesDir).filter((p) => p.endsWith(ext));

module.exports = Object.fromEntries(ruleFiles.map((p) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a magic filename for the plugin?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

eslint-local-rules.js is, yeah. Unfortunately, it doesn't allow you to use a dotted name. You can have eslint-local-rules.js or eslint-local-rules/index.js, and that's it.

I could switch to https://github.com/taskworld/eslint-plugin-local, which has more downloads is literally just module.exports = require('../../.eslintplugin'), a better filename.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Switched it; it hasn't been updated in 5 years but when it's so simple... :D

@jakebailey jakebailey changed the title Replace eslint rulesdir with eslint-plugin-local-rules, convert eslint rules to JS Replace eslint rulesdir with eslint-plugin-local, convert eslint rules to JS Aug 22, 2022
.yarnrc
tmp
.eslintplugin.js
.eslintcache
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I should have included this on my eslint-at-root PR; I really dislike npmignore for this reason. Maybe I'll sit down and change our package to use "files" instead.

// Rename this file 'settings.json' or merge its
// contents into your existing settings.
{
"eslint.validate": [
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

With this gone, the template consists solely of suggestions for typescript.tsdk values.

@jakebailey
Copy link
Copy Markdown
Member Author

Should should be ready for review; I don't think I have any other changes.

@jakebailey jakebailey merged commit 6362fb2 into microsoft:main Aug 22, 2022
@jakebailey jakebailey deleted the convert-eslint-rules branch August 22, 2022 20:46
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants