Update: compatibility with eslint-mdx #178

JounQin · 2021-03-12T17:34:55Z

resolve #134, fix #176 and mdx-js/eslint-mdx#259

close #180

I added a getShortLang helper, but I don't know if you'll agree. Personally I think it's useful for example I'm writing JavaScript language, but the file extension should be .js. Or, we can provide an option.

See also #178 (comment)

eslint-github-bot · 2021-03-12T17:34:59Z

Hi @JounQin!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag must be one of the following:

The Tag is one of the following:
- Fix - for a bug fix.
- Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
- New - implements a new feature.
- Breaking - for a backwards-incompatible enhancement or feature.
- Docs - changes to documentation only.
- Build - changes to build process only.
- Upgrade - for a dependency upgrade.
- Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).
You can use the labels of the issue you are working on to determine the best tag.
There should be a space following the initial tag and colon, for example 'New: Message'.
The first letter of the tag should be in uppercase

Read more about contributing to ESLint here

resolve #134

JounQin · 2021-03-12T17:37:17Z

.eslintrc.js

+            "extends": ["plugin:mdx/recommended"],
+            "processor": "markdown/markdown",
+            "rules": {
+                "mdx/remark": 0


The related PR has not been merged and released, disable temporarily.

JounQin · 2021-03-12T17:39:11Z

lib/processor.js

@@ -16,7 +16,7 @@ const SUPPORTS_AUTOFIX = true;

 const markdown = unified().use(remarkParse);

-let blocks = [];


The blocks will not work when writing markdown codes in code blocks.

Lifecycle:

preprocess('.md') -> preprocess('.md/0_0.md') -> postprocess('.md/0_0.md') -> postprocess('.md')

This was not a problem before because there was no markdown -> md mapper before, so .markdown files were not linted actually.

JounQin · 2021-03-12T17:41:10Z

package.json

    "eslint-plugin-node": "^9.0.0",
    "eslint-release": "^3.1.2",
    "mocha": "^6.2.2",
    "nyc": "^14.1.1"
  },
  "dependencies": {
-    "remark-parse": "^5.0.0",
-    "unified": "^6.1.2"
+    "remark-parse": ">=5.0.0 <9.0.0",


9.0 has breaking change, its parser is rewritten totally.

JounQin · 2021-03-13T03:57:30Z

friendly ping @btmills @mdjermanovic

btmills · 2021-03-18T04:56:04Z

Hi @JounQin, I appreciate the enthusiasm! #134 was closed because MDX was out of scope for this plugin, which is focused just on Markdown. If something significant has changed since then, we can reconsider if you'd like to open a new issue describing how the situation is different now. (Maybe that's #179? Having a clear problem statement and situation update would help.) I'll hold off reviewing this for now because I wouldn't want you to put a bunch of effort into a feature that may not be added. If we decide on the issue to move forward, then we can come back here and discuss how best to implement the feature.

JounQin · 2021-03-18T05:05:38Z

@btmills Hi, #179 is not related, at this PR actually only adds blocksCache and getShortLang and better TypeScript support.

For blocksCache, it is actually an issue described at #178 (comment)

For getShortLang, it fixes #176

For ts typings generated by typescript + jsdoc, it will be easier to be resued in eslint-mdx. I can just copy-paste the source code of eslint-plugin-markdown into https://github.com/mdx-js/eslint-mdx/blob/master/packages/eslint-plugin-mdx/src/processors/markdown.ts for now.

I added eslint-plugin-mdx as dev dep just to make sure they are working compatibly.

btmills · 2021-03-18T05:34:58Z

I see, there's a lot going on here. Let's split it up:

For blocksCache, it is actually an issue described at #178 (comment)

Can you open an issue describing this in detail? If it's a bug in the current version, that's worth fixing, but I want to dig in first to make sure I understand.

For getShortLang, it fixes #176

I believe #176 is working as intended by eslint/rfcs#3. I'll wait a bit longer for the author to respond just in case there really is a bug. If not, I'll close that issue. Adding the language mapper would be a breaking change. In v2, the plugin just moved away from hard-coding extensions like v1 had to. That was all enabled by the new ESLint processor API from RFC3.

For ts typings generated by typescript + jsdoc

I see you improved the JSDoc types. That's great! However, projects under the ESLint org don't ship types as part of the package. So we'll gratefully accept any JSDoc annotation improvements you find as standalone Docs: pull requests in order to make it easier for you to generate types as a consumer, but generating and publishing types (other than JSDoc) inside of this package is out of scope.

I added eslint-plugin-mdx as dev dep just to make sure they are working compatibly.

Now I understand what that's for. The repository already has example setups with eslint-plugin-react and @typescript-eslint in the examples/ directory, and those get run as part of our test suite. Can you move that there instead? That way it'll still work as a compatibility test while also serving as documentation for people who want to use eslint-plugin-markdown and eslint-plugin-mdx together.

JounQin · 2021-03-18T05:48:45Z

Can you open an issue describing this in detail? If it's a bug in the current version, that's worth fixing, but I want to dig in first to make sure I understand.

raised #181 to track

I believe #176 is working as intended by eslint/rfcs#3. I'll wait a bit longer for the author to respond just in case there really is a bug. If not, I'll close that issue. Adding the language mapper would be a breaking change. In v2, the plugin just moved away from hard-coding extensions like v1 had to. That was all enabled by the new ESLint processor API from RFC3.

I understand your position, but IMO the problem is that should we treat code block with JavaScript lang as .js files to be linted, I think it should be much more expected for users and will be easier for users to config. Or a setting of language mapper can be provided with true implied as the default language mapper.

I will refactor other things you mentioned.

JounQin · 2021-03-18T10:47:34Z

If you don't like language mapper support, I'll drop it here and add this support on eslint-plugin-mdx side.

close this PR in favor of the splitted two.

https://github.com/eslint/eslint-plugin-markdown/pull/178\#issuecomment-801821377

JounQin · 2021-03-23T02:57:58Z

@btmills Any news for the two separated PRs?

Update: compatibility with eslint-mdx

fc770f3

resolve #134

JounQin commented Mar 12, 2021

View reviewed changes

JounQin changed the title ~~feat: compatibility with eslint-mdx~~ Update: compatibility with eslint-mdx Mar 12, 2021

feat: improve jsdoc types, generate types from jsdoc

3612951

JounQin mentioned this pull request Mar 13, 2021

eslint-plugin-mdx won't work along with eslint-plugin-markdown mdx-js/eslint-mdx#259

Closed

This was referenced Mar 13, 2021

feat: use eslint-plugin-markdown as processor! mdx-js/eslint-mdx#283

Merged

eslint-plugin-markdown v2 doesn't lint code samples with javascript syntax #176

Closed

JounQin mentioned this pull request Mar 18, 2021

[discussion] compatibility with other plugins related to remark #180

Closed

JounQin mentioned this pull request Mar 18, 2021

[bug] blocks will be reset on preprocess every time and will break .md in .md #181

Closed

This was referenced Mar 18, 2021

Docs: improve jsdoc, better for typings #182

Merged

Fix: use blocksCache instead of single blocks instance (fixes #181) #183

Merged

JounQin closed this Mar 18, 2021

JounQin deleted the feat/mdx branch March 18, 2021 10:47

JounQin referenced this pull request in mdx-js/eslint-mdx Mar 18, 2021

refactor: move getShortLang outside markdown

b597720

https://github.com/eslint/eslint-plugin-markdown/pull/178\#issuecomment-801821377

JounQin referenced this pull request in mdx-js/eslint-mdx Mar 18, 2021

refactor: move getShortLang outside markdown processor

3234599

https://github.com/eslint/eslint-plugin-markdown/pull/178\#issuecomment-801821377

JounQin mentioned this pull request Mar 18, 2021

refactor: move getShortLang outside markdown processor mdx-js/eslint-mdx#291

Merged

JounQin referenced this pull request in mdx-js/eslint-mdx Mar 18, 2021

refactor: move getShortLang outside markdown processor

2925af7

https://github.com/eslint/eslint-plugin-markdown/pull/178\#issuecomment-801821377

JounQin referenced this pull request in mdx-js/eslint-mdx Mar 19, 2021

refactor: move getShortLang outside markdown processor

948caa7

https://github.com/eslint/eslint-plugin-markdown/pull/178\#issuecomment-801821377

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update: compatibility with eslint-mdx #178

Update: compatibility with eslint-mdx #178

JounQin commented Mar 12, 2021 •

edited

Loading

eslint-github-bot bot commented Mar 12, 2021

JounQin Mar 12, 2021

JounQin Mar 12, 2021 •

edited

Loading

JounQin Mar 12, 2021

JounQin commented Mar 13, 2021

btmills commented Mar 18, 2021

JounQin commented Mar 18, 2021 •

edited

Loading

btmills commented Mar 18, 2021

JounQin commented Mar 18, 2021 •

edited

Loading

JounQin commented Mar 18, 2021

JounQin commented Mar 23, 2021 •

edited

Loading

		@@ -16,7 +16,7 @@ const SUPPORTS_AUTOFIX = true;

		const markdown = unified().use(remarkParse);

		let blocks = [];

Update: compatibility with eslint-mdx #178

Update: compatibility with eslint-mdx #178

Conversation

JounQin commented Mar 12, 2021 • edited Loading

eslint-github-bot bot commented Mar 12, 2021

JounQin Mar 12, 2021

Choose a reason for hiding this comment

JounQin Mar 12, 2021 • edited Loading

Choose a reason for hiding this comment

JounQin Mar 12, 2021

Choose a reason for hiding this comment

JounQin commented Mar 13, 2021

btmills commented Mar 18, 2021

JounQin commented Mar 18, 2021 • edited Loading

btmills commented Mar 18, 2021

JounQin commented Mar 18, 2021 • edited Loading

JounQin commented Mar 18, 2021

JounQin commented Mar 23, 2021 • edited Loading

JounQin commented Mar 12, 2021 •

edited

Loading

JounQin Mar 12, 2021 •

edited

Loading

JounQin commented Mar 18, 2021 •

edited

Loading

JounQin commented Mar 18, 2021 •

edited

Loading

JounQin commented Mar 23, 2021 •

edited

Loading