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

JounQin · 2021-03-12T16:40:27Z

This PR uses a different way to lint code blocks: use eslint-plugin-markdown as processor directly!

What means we do not need to maintain related codes by ourselves, I raised another PR eslint/markdown#178 for eslint-plugin-markdown to integrate with eslint-plugin-mdx better, close #282.

close #278

ChristianMurphy · 2021-03-13T04:04:21Z

packages/eslint-plugin-mdx/src/processors/options.ts

+  path =>
+    path.endsWith('/eslint/lib/linter/linter.js') ||
+    path.endsWith('\\eslint\\lib\\linter\\linter.js'),
+)


this seems a bit fragile?
Would it make sense to use something like require.resolve?

I'm not sure, it's based on https://github.com/sveltejs/eslint-plugin-svelte3/blob/master/src/processor_options.js

If we use require.resolve, it will always be added into require.cache, so I don't know is that correct.

I tried:

let resolvedLinterPath: string // find Linter instance const linterPath = Object.keys(require.cache).find( path => path === resolvedLinterPath || (resolvedLinterPath = require.resolve('eslint/lib/linter/linter')), )

And there are an error:

(node:46681) Warning: Accessing non-existent property 'Linter' of module exports inside circular dependency (Use `node --trace-warnings ...` to show where the warning was created) Oops! Something went wrong! :( ESLint: 7.21.0 TypeError: Cannot read config file: /Users/JounQin/Workspaces/GitHub/eslint-mdx/node_modules/@1stg/eslint-config/base.js Error: Cannot read property 'prototype' of undefined Referenced from: /Users/JounQin/Workspaces/GitHub/eslint-mdx/node_modules/@1stg/eslint-config/index.js at Object.<anonymous> (/Users/JounQin/Workspaces/GitHub/eslint-mdx/packages/eslint-plugin-mdx/src/processors/options.ts:33:27) at Module._compile (/Users/JounQin/Workspaces/GitHub/eslint-mdx/node_modules/v8-compile-cache/v8-compile-cache.js:192:30) at Module.m._compile (/Users/JounQin/Workspaces/GitHub/eslint-mdx/node_modules/ts-node/src/index.ts:1056:23) at Module._extensions..js (internal/modules/cjs/loader.js:1092:10) at Object.require.extensions.<computed> [as .ts] (/Users/JounQin/Workspaces/GitHub/eslint-mdx/node_modules/ts-node/src/index.ts:1059:12) at Module.load (internal/modules/cjs/loader.js:928:32) at Function.Module._load (internal/modules/cjs/loader.js:769:14) at Module.require (internal/modules/cjs/loader.js:952:19) at require (/Users/JounQin/Workspaces/GitHub/eslint-mdx/node_modules/v8-compile-cache/v8-compile-cache.js:159:20) at Object.<anonymous> (/Users/JounQin/Workspaces/GitHub/eslint-mdx/packages/eslint-plugin-mdx/src/processors/remark.ts:6:1)

codecov-io · 2021-03-13T04:07:09Z

Codecov Report

Merging #283 (ea32dac) into master (5052eb8) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #283   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        22    +1     
  Lines          464       538   +74     
  Branches        90       111   +21     
=========================================
+ Hits           464       538   +74

Impacted Files	Coverage Δ
.eslintrc.js	`100.00% <ø> (ø)`
packages/eslint-plugin-mdx/src/configs/base.ts	`100.00% <ø> (ø)`
packages/eslint-plugin-mdx/src/configs/helpers.ts	`100.00% <ø> (ø)`
...kages/eslint-plugin-mdx/src/configs/recommended.ts	`100.00% <ø> (ø)`
...lint-plugin-mdx/src/rules/no-unescaped-entities.ts	`100.00% <ø> (ø)`
...lint-plugin-mdx/src/rules/no-unused-expressions.ts	`100.00% <ø> (ø)`
test/helpers.ts	`100.00% <ø> (ø)`
packages/eslint-mdx/src/helpers.ts	`100.00% <100.00%> (ø)`
packages/eslint-mdx/src/index.ts	`100.00% <100.00%> (ø)`
packages/eslint-mdx/src/parser.ts	`100.00% <100.00%> (ø)`
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5052eb8...ea32dac. Read the comment docs.

JounQin · 2021-03-13T04:51:35Z

@wooorm Ready for review and merge!

I just copied eslint-plugin-markdown codes temporarily, will use it as dependency once eslint/markdown#178 merged.

wooorm

What’s the benefit of copy-pasting eslint-plugin-markdown into this project?
Assuming you could use it here as a depenedncy and not copy-paste, still: what’s the benefit of supporting both markdown and MDX here?

wooorm · 2021-03-13T08:32:52Z

packages/eslint-plugin-mdx/src/processors/markdown.ts

+const LANGUAGES_MAPPER: Record<string, string> = {
+  javascript: 'js',
+  javascriptreact: 'jsx',
+  typescript: 'ts',
+  typescriptreact: 'tsx',
+  markdown: 'md',
+  mdown: 'md',
+  mkdn: 'md',
+}


Where are you basic JS + React, and TS + React off? They don’t work:

```text/javascript console.log(1) ``` ```javascript/react console.log(1) ``` ```javascript console.log(1) ``` ```js console.log(1) ``` ```.js console.log(1) ``` ```JS console.log(1) ```

console.log(1)

console.log(1)

console.log(1)

console.log(1)

console.log(1)

console.log(1)

I think the get short language functionality seems more complex that needed? This seems closer: (node.lang || '').toLowerCase().replace(/\./g, '')

It will generate a virtual filename for code blocks. What means:

javascript -> 0_0.javascript

So I add this mapper so that the virtual files will be linted correctly by ESLint again.

When I try javascript/react or text/javascript, etc in VSCode, there will be no syntax highlighting, so I don't think this is a standard usage.

Of course, I can add support for it, maybe.

Of course, I can add support for it, maybe.

No, I don’t think you should!

But where did you get getShortLang from?

It seems to do much more than needed

But where did you get getShortLang from?

I wrote it by myself.

It seems to do much more than needed

Why? Writing JavaScript or typescript as lang for code blocks is common, and we need to support linting them by default.

@wooorm See also eslint/markdown#176

JounQin · 2021-03-13T08:34:10Z

What’s the benefit of copy-pasting eslint-plugin-markdown into this project?

Assuming you could use it here as a depenedncy and not copy-paste, still: what’s the benefit of supporting both markdown and MDX here?

It's just temporarily, I've mentioned that

will use it as dependency once eslint/markdown#178 merged.

wooorm · 2021-03-13T08:36:15Z

My second point talks about after it’s temporary

JounQin · 2021-03-13T08:36:49Z

My second point talks about after it’s temporary

I'm not sure to understand correctly. eslint-plugin-markdown aims to lint code blocks both in md or mdx files.

wooorm · 2021-03-13T08:38:13Z

Ok, so you are only using it to check codeblocks in MDX files, correct?

Put another way, it’s NOT that eslint-mdx now supports .md files?

JounQin · 2021-03-13T08:43:48Z

Ok, so you are only using it to check codeblocks in MDX files, correct?

Put another way, it’s NOT that eslint-mdx now supports .md files?

eslint-mdx supports md files for a long time with remark-lint!

With help of eslint-plugin-markdown, we support codeblocks too now.

eslint-plugin-markdown can only lint code blocks while eslint-plugin-mdx can lint markdown content + ES syntaxes.

wooorm · 2021-03-13T08:51:12Z

With help of eslint-plugin-markdown, we support codeblocks too now.

I’m all for that!

eslint-mdx supports md files for a long time with remark-lint!

I’m all for supporting remark lint rules here, but I’m not for supporting markdown files here: MDX !== MD (https://github.com/wooorm/xdm#caveats).
I think markdown belongs in eslint-plugin-markdown and MDX belongs here. If there’s overlap internally (such as what you’re doing here with code blocks), that‘s fine.

From a users perspective: there are markdown users that don’t use MDX; they might want to check their markdown too, so they find eslint-plugin-markdown. They shouldn’t have to install eslint-mdx.

JounQin · 2021-03-13T08:57:11Z

I’m all for supporting remark lint rules here, but I’m not for supporting markdown files here: MDX !== MD

remark-mdx is not enabled for md files, so that's OK.

See https://github.com/mdx-js/eslint-mdx/blob/master/packages/eslint-mdx/src/parser.ts#L170

I think markdown belongs in eslint-plugin-markdown and MDX belongs here.

Well, eslint-plugin-markdown does not support remark-lint at all right now.

If we want to lint markdown syntaxes in mdx files, it should be done in eslint-plugin-mdx, so the md files support is incidental result.

wooorm · 2021-03-13T09:03:23Z

remark-mdx is not enabled for md files, so that's OK.

Great! I was worried about that, and your “eslint-mdx supports md files for a long time with remark-lint!” seemed to indicate that, hence my asking.

Well, eslint-plugin-markdown does not support remark-lint at all right now.

Yep, might be something to upstream to them, if they’re interested!

JounQin · 2021-03-13T09:19:02Z

Yep, might be something to upstream to them, if they’re interested!

I don't know, maybe another discussion in eslint-plugin-markdown will help.

rasied eslint/markdown#179

JounQin · 2021-03-13T10:22:05Z

@wooorm Can we merge and release then?

JounQin · 2021-03-13T11:55:51Z

@wooorm Please help to review last once. I'll focus on remark-mdx@next after this PR been merged and released!

JounQin · 2021-03-14T14:18:48Z

I'm going to merge first, @wooorm please help to release.

This PR and #281 are both great to have.

JounQin · 2021-03-15T17:26:51Z

@wooorm Any time to release?

wooorm · 2021-03-16T09:28:15Z

Done!

P.S. I get a ton of issues and notifications, and some days I try and focus on other things. So, you don’t have to ping me every day. I’ll get to it! If I forget for several days, then of course feel free to ping me!

feat: use eslint-plugin-markdown as processor!

60cb5d1

close #278

This was referenced Mar 12, 2021

feat: experimentally support linting code blocks #282

Closed

Update: compatibility with eslint-mdx eslint/markdown#178

Closed

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

Closed

JounQin force-pushed the feat/eslint-plugin-markdown branch from e5a69c5 to 3ff1cc9 Compare March 13, 2021 04:00

refactor: markdown.js -> markdown.ts

9a3a99b

ChristianMurphy reviewed Mar 13, 2021

View reviewed changes

JounQin force-pushed the feat/eslint-plugin-markdown branch from 3ff1cc9 to 9a3a99b Compare March 13, 2021 04:04

JounQin self-assigned this Mar 13, 2021

JounQin added 🦋 type/enhancement This is great to have 🧒 semver/minor This is backwards-compatible change labels Mar 13, 2021

refactor: remove unused functions

e0d6ed4

JounQin force-pushed the feat/eslint-plugin-markdown branch from 3304055 to e0d6ed4 Compare March 13, 2021 04:42

JounQin requested a review from wooorm March 13, 2021 04:50

docs: @babel/eslint-parser is used automatically too

51ba6b3

wooorm reviewed Mar 13, 2021

View reviewed changes

refactor: use import type for ts files

19a9b5d

JounQin mentioned this pull request Mar 13, 2021

[feat] support linting markdown syntaxes like eslint-plugin-mdx via remark-lint eslint/markdown#179

Closed

JounQin force-pushed the feat/eslint-plugin-markdown branch from 4414ac0 to 50251ff Compare March 13, 2021 10:39

refactor: simplify postprocess and verify types

3789807

JounQin force-pushed the feat/eslint-plugin-markdown branch 2 times, most recently from 79fc2c2 to 9269549 Compare March 13, 2021 11:45

docs: notice how to disable or change severity of remark rules

2934abf

JounQin force-pushed the feat/eslint-plugin-markdown branch from 9269549 to 2934abf Compare March 13, 2021 11:48

chore: bump config deps

ea32dac

JounQin force-pushed the feat/eslint-plugin-markdown branch from 6b4e8b4 to ea32dac Compare March 14, 2021 14:11

JounQin merged commit abe30cb into master Mar 14, 2021

JounQin deleted the feat/eslint-plugin-markdown branch March 14, 2021 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

JounQin commented Mar 12, 2021 •

edited

Loading

ChristianMurphy Mar 13, 2021

JounQin Mar 13, 2021

JounQin Mar 13, 2021

JounQin Mar 13, 2021

codecov-io commented Mar 13, 2021 •

edited

Loading

JounQin commented Mar 13, 2021

wooorm left a comment

wooorm Mar 13, 2021

JounQin Mar 13, 2021

wooorm Mar 13, 2021

JounQin Mar 13, 2021 •

edited

Loading

JounQin Mar 13, 2021

JounQin commented Mar 13, 2021 •

edited

Loading

wooorm commented Mar 13, 2021

JounQin commented Mar 13, 2021 •

edited

Loading

wooorm commented Mar 13, 2021

JounQin commented Mar 13, 2021 •

edited

Loading

wooorm commented Mar 13, 2021

JounQin commented Mar 13, 2021 •

edited

Loading

wooorm commented Mar 13, 2021

JounQin commented Mar 13, 2021 •

edited

Loading

JounQin commented Mar 13, 2021

JounQin commented Mar 13, 2021

JounQin commented Mar 14, 2021

JounQin commented Mar 15, 2021

wooorm commented Mar 16, 2021

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

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

Conversation

JounQin commented Mar 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Mar 13, 2021 • edited Loading

Codecov Report

JounQin commented Mar 13, 2021

wooorm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JounQin Mar 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JounQin commented Mar 13, 2021 • edited Loading

wooorm commented Mar 13, 2021

JounQin commented Mar 13, 2021 • edited Loading

wooorm commented Mar 13, 2021

JounQin commented Mar 13, 2021 • edited Loading

wooorm commented Mar 13, 2021

JounQin commented Mar 13, 2021 • edited Loading

wooorm commented Mar 13, 2021

JounQin commented Mar 13, 2021 • edited Loading

JounQin commented Mar 13, 2021

JounQin commented Mar 13, 2021

JounQin commented Mar 14, 2021

JounQin commented Mar 15, 2021

wooorm commented Mar 16, 2021

JounQin commented Mar 12, 2021 •

edited

Loading

codecov-io commented Mar 13, 2021 •

edited

Loading

JounQin Mar 13, 2021 •

edited

Loading

JounQin commented Mar 13, 2021 •

edited

Loading

JounQin commented Mar 13, 2021 •

edited

Loading

JounQin commented Mar 13, 2021 •

edited

Loading

JounQin commented Mar 13, 2021 •

edited

Loading

JounQin commented Mar 13, 2021 •

edited

Loading