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

docs: Update Create a Plugin for flat config #17826

Merged
merged 10 commits into from Dec 27, 2023
Merged

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Dec 7, 2023

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[x] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Updated the plugin documentation to be the correct format for flat config.

Refs #13481

Is there anything you'd like reviewers to focus on?

Did I miss anything?

Copy link

netlify bot commented Dec 7, 2023

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 8c3b64f
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/658b6725db702a0008fa2daf
😎 Deploy Preview https://deploy-preview-17826--docs-eslint.netlify.app/extend/custom-processors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nzakas
Copy link
Member Author

nzakas commented Dec 7, 2023

@christian-bromann looks like browser test is failing again. :(

@christian-bromann
Copy link
Contributor

@nzakas I just reverted changes we released 10h ago, should be all good again.

@nzakas
Copy link
Member Author

nzakas commented Dec 7, 2023

@christian-bromann thanks, looks like it's working again. 🙏

Copy link

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

@github-actions github-actions bot added the Stale label Dec 17, 2023
@nzakas nzakas marked this pull request as ready for review December 20, 2023 23:01
@nzakas nzakas requested a review from a team as a code owner December 20, 2023 23:01
@nzakas
Copy link
Member Author

nzakas commented Dec 20, 2023

@eslint/eslint-team this is ready for review.

@nzakas nzakas changed the title docs!: Update Create a Plugin for flat config docs: Update Create a Plugin for flat config Dec 21, 2023
@eslint-github-bot eslint-github-bot bot added the documentation Relates to ESLint's documentation label Dec 21, 2023
@nzakas nzakas added breaking This change is backwards-incompatible and removed Stale labels Dec 21, 2023

1. `globals` - acts the same `globals` in a configuration file. The keys are the names of the globals and the values are `true` to allow the global to be overwritten and `false` to disallow.
1. `parserOptions` - acts the same as `parserOptions` in a configuration file.
Plugins can expose custom rules for use in ESLint. To do so, the plugin must export a `rules` object containing a key-value mapping of rule ID to rule. The rule ID does not have to follow any naming convention (so it can just be `dollar-sign`, for instance). To learn more about creating custom rules in plugins, refer to [Custom Rules](custom-rules).
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should mention that the use of / in rule ID is not supported because #17901 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Looks like parsing is different when the plugin doesn't start with @ (i.e., the plugin is not scoped), so / in rule ID would work in that case.

// mimic scoped npm packages
if (ruleId.startsWith("@")) {
pluginName = ruleId.slice(0, ruleId.lastIndexOf("/"));
} else {
pluginName = ruleId.slice(0, ruleId.indexOf("/"));
}

// this works
module.exports = {
    plugins: {
        "foo": {
            rules: {
                "bar/baz": {
                    create() {
                        return {}
                    }
                }
            }
        }
    },
    rules: {
        "foo/bar/baz": 2
    }
};

// this throws TypeError: Key "rules": Key "@foo/bar/baz": Could not find plugin "@foo/bar"
module.exports = {
    plugins: {
        "@foo": {
            rules: {
                "bar/baz": {
                    create() {
                        return {}
                    }
                }
            }
        }
    },
    rules: {
        "@foo/bar/baz": 2
    }
};

Nevertheless, it does seem best to generally say that rule IDs should not contain /, to avoid ambiguity.

Copy link
Member Author

@nzakas nzakas Dec 26, 2023

Choose a reason for hiding this comment

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

Yeah, I needed to do this to mimic the behavior of eslintrc plugin names. I've added a note about the slash.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also added a note about the @ restrictions in namespaces.

docs/src/extend/plugins.md Outdated Show resolved Hide resolved
docs/src/extend/plugins.md Show resolved Hide resolved
docs/src/extend/plugins.md Outdated Show resolved Hide resolved
docs/src/extend/plugins.md Outdated Show resolved Hide resolved
docs/src/extend/plugins.md Outdated Show resolved Hide resolved
nzakas and others added 5 commits December 26, 2023 13:45
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
```

### Configs in Plugins
::: warning
Namespaces that don't begin with `@` don't have any restrictions; namespaces that begin with `@` must also contain a `/`. For example, `@eslint` is not a valid namespace but `@eslint/plugin` is valid. This restriction is for backwards compatibility with eslintrc plugin naming restrictions.
Copy link
Member

Choose a reason for hiding this comment

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

I think this doesn't reflect the current restrictions in flat config.

Namespaces that don't begin with @ have a restriction that they must not contain a /.

// this throws: TypeError: Key "rules": Key "foo/bar/my-rule": Could not find plugin "foo".
module.exports = [{
    plugins: {
        "foo/bar": {
            rules: {
                "my-rule": {
                    create() {
                        return {};
                    }
                }
            }
        }
    },
    rules: {
        "foo/bar/my-rule": 2,
    }
}];

Namespaces that begin with @ don't have any restrictions (except that their rule names definitely cannot contain /, but we recommend that for all rules regardless of the namespace). These namespaces can contain / but aren't required to. As for the backwards compatibility, @typecript-eslint, for instance, was a valid namespace in eslintrc (rules are in the form @typescript-eslint/rule-name).

// this all works
module.exports = [{
    plugins: {
        "@foo": {
            rules: {
                "my-rule": {
                    create() {
                        return {};
                    }
                }
            }
        },
        "@foo/bar": {
            rules: {
                "my-rule": {
                    create() {
                        return {};
                    }
                }
            }
        }
    },
    rules: {
        "@foo/my-rule": 2,
        "@foo/bar/my-rule": 2
    }
}];

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah good call. My brain is a bit fried from reading and writing all this documentation.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit fdf0424 into main Dec 27, 2023
17 checks passed
@mdjermanovic mdjermanovic deleted the flat-plugin-docs branch December 27, 2023 07:57
bmish added a commit to bmish/eslint that referenced this pull request Dec 27, 2023
* main:
  docs: Migrate to v9.0.0 (eslint#17905)
  docs: Switch to flat config by default (eslint#17840)
  docs: Update Create a Plugin for flat config (eslint#17826)
  feat!: add two more cases to `no-implicit-coercion` (eslint#17832)
  docs: Switch shareable config docs to use flat config (eslint#17827)
  chore: remove creating an unused instance of Linter in tests (eslint#17902)
  feat!: Switch Linter to flat config by default (eslint#17851)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change is backwards-incompatible documentation Relates to ESLint's documentation
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

None yet

4 participants