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

Add an option to specify different rules inside a markdown file #170

Closed
aduh95 opened this issue Feb 2, 2021 · 7 comments
Closed

Add an option to specify different rules inside a markdown file #170

aduh95 opened this issue Feb 2, 2021 · 7 comments

Comments

@aduh95
Copy link

aduh95 commented Feb 2, 2021

I'm trying to set up a rule set that is able to discriminate between ESM examples and CJS code snippets inside the markdown documentation:

```js esm
export default {};
```
```js cjs
'use strict';
module.exports = {};
```

The issue is that only the js is passed through unfied lang data, and the part after the space is pass to the meta option:

{
  type: 'html',
  lang: 'js',
  meta: 'esm',
  value: 'export default {};',
  position: {
    start: { line: 1, column: 1, offset: 0 },
    end: { line: 3, column: 4, offset: 32 }
  }
}
{
  type: 'html',
  lang: 'js',
  meta: 'cjs',
  value: "'use strict';\nmodule.exports = {};",
  position: {
    start: { line: 4, column: 1, offset: 33 },
    end: { line: 7, column: 4, offset: 81 }
  }
}

I wish there was a way to create separate rules for those blocks, something like:

// .eslintrc.js
module.exports = {
    plugins: ["markdown"],
    overrides: [
        {
            files: ["**/*.md/*.js:cjs"],
            parserOption: { sourceType: "script" },
            rules: {
                // ...
            }
        },
        {
            files: ["**/*.md/*.js:esm"],
            parserOption: { sourceType: "module" },
            rules: {
                // ...
            }
        },
    ]
};

EDIT: I'm using remark-parse 9.0.0, the meta property doesn't seem to be available in remark-parse 5.x this repo is using.

aduh95 added a commit to aduh95/eslint-plugin-markdown that referenced this issue Feb 4, 2021
@btmills
Copy link
Member

btmills commented Feb 11, 2021

Are you able to use ```cjs and ```mjs fences instead of ```js? That would allow you to configure **/*.md/*.cjs and **/*.md/*.mjs separately.

@aduh95
Copy link
Author

aduh95 commented Feb 11, 2021

Sure, but that doesn't address the underlining issue: I'd still want to have the option to specify different set of overrides for a same language. Ignoring those meta strings may be fine for some people, but I think it can be a very useful feature for others.

If we want to make a breaking change, I think now (I.E. before v2 is released) would be the right time as folks upgrading to v2 are going too have to rewrite their ESLint configuration anyway.
If such a breaking change is not acceptable, maybe we could make this configurable/opt-in?

@btmills
Copy link
Member

btmills commented Feb 14, 2021

While I agree switching functionality based on the meta string is neat, there'd need to be some important use case for which that route is the only option. One downside is that it claims exclusive use of the meta string. What if there are other tools that want to use it as well?

Let's step back from the specific implementation idea. What use case are you trying to achieve?

I think now (I.E. before v2 is released) would be the right time

v2 was already a release candidate when you opened this issue, so any breaking change would have to wait for a hypothetical v3.

@aduh95
Copy link
Author

aduh95 commented Feb 15, 2021

One downside is that it claims exclusive use of the meta string.

I'm not sure that's true, you can easily configure the plugin to ignore it – in the implementation I made in #172 at least:

            "files": [
                "**/*.{md,mkdn,mdown,markdown}/*.{js,javascript,jsx,node}",
                "**/*.{md,mkdn,mdown,markdown}/*.{js,javascript,jsx,node}:*"
            ]

It's too bad ESLint doesn't allow users to pass a pluginOptions object to introduce that kind of changes in a non-breaking way.

What use case are you trying to achieve?

I'm working on this PR: #172
Like I said in the OP, I would like to add different rule sets for both blocks, and the current rules on that repo forbid to use cjs or mjs as language descriptor (warning Incorrect code language flag fenced-code-flag remark-lint).
Also, I can imagine we want to introduce more specific rule sets in the future.

v2 was already a release candidate when you opened this issue, so any breaking change would have to wait for a hypothetical v3.

Hum OK I didn't realize v2 was already feature frozen. I still think it would still be a neat feature to have, so maybe for later?

aduh95 added a commit to aduh95/remark-preset-lint-node that referenced this issue Feb 23, 2021
aduh95 added a commit to aduh95/remark-preset-lint-node that referenced this issue Feb 23, 2021
Trott pushed a commit to nodejs/remark-preset-lint-node that referenced this issue Mar 4, 2021
@btmills
Copy link
Member

btmills commented May 1, 2021

the current rules on that repo forbid to use cjs or mjs as language descriptor (warning Incorrect code language flag fenced-code-flag remark-lint)

It sounds like the ideal fix is for remark-lint to allow cjs and mjs in info strings, and this change would be a workaround for that solution. Are you able to configure remark-lint that way, and if so, does that solve your use case for now?

@aduh95
Copy link
Author

aduh95 commented May 1, 2021 via email

@btmills
Copy link
Member

btmills commented May 1, 2021

Great! Closing since this has been solved.

@btmills btmills closed this as completed May 1, 2021
Marlyfleitas added a commit to Marlyfleitas/Node-remark-preset-lint that referenced this issue Aug 26, 2022
patrickm68 added a commit to patrickm68/Node-preset-lint that referenced this issue Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@btmills @aduh95 and others