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

Regression of stable/reliable run since v1.12 #339

Closed
codejedi365 opened this issue Aug 25, 2021 · 14 comments
Closed

Regression of stable/reliable run since v1.12 #339

codejedi365 opened this issue Aug 25, 2021 · 14 comments
Labels
🙋 no/question This does not need any changes

Comments

@codejedi365
Copy link

Subject of the issue

Semver Minor upgrades have made functionality & performance worse over the last four versions. Yesterday, I went to run the linter after trashing package-lock.json and re-installing the updates, I found npm run lint hitting a runtime error. I incrementally rolled back the updates and found none of the latest versions are stable or working well.

In summary I found:

  • v1.14 - v1.15: TypeError: Cannot read property of 'indent' of undefined
  • v1.13.0: Infinite Loop (never)
  • v1.12.0: Terrible Performance but finishes eventually (45sec)
  • v1.11.0: BEST PERFORMANCE (7sec)

I isolated the issue to be this plugin via .eslintrc.js as I commented out the plugin:mdx/recommended and everything works if it doesn't pass through this plugin.

Your environment

  • OS: Mac OS v11.5

  • Packages:

    • @typescript-eslint/eslint-plugin@^4.29.1
    • @typescript-eslint/parser@^4.29.1
    • eslint@^7.32.0
    • eslint-config-airbnb-base@^14.2.1
    • eslint-config-airbnb-typescript@^12.3.1
    • eslint-config-prettier@^8.3.0
    • eslint-plugin-import@^2.24.0
    • eslint-plugin-jest@^24.4.0
    • eslint-plugin-json-format@^2.0.1
    • eslint-plugin-mdx@~1.11.0
    • eslint-plugin-prettier@^3.4.0
    • eslint-plugin-mdx@~1.11.0
    • eslint-plugin-prettier@^3.4.0
    • prettier@^2.3.2
    • remark-lint-alphabetize-lists@^3.0.0
    • remark-lint-no-dead-urls@^1.1.0
    • remark-preset-lint-consistent@^4.0.0
    • remark-preset-lint-markdown-style-guide@^4.0.0
    • remark-preset-lint-recommended@^5.0.0
    • remark-preset-prettier@^0.5.1
    • ts-loader@^9.2.5
    • typescript@^4.3.5
  • Env: Node LTS v12 & Node LTS v14, [email protected]

Steps to reproduce

# 0. configure pre-req of NodeJs v12 & v14.  Use NVM to swap versions quickly.
# 1. Clone my repo
$ git clone https://github.com/codejedi365/treemap-js.git

# 2. Checkout commit
$ git checkout -b lint-test bd37e75f

# 3. Activate node version & upgrade npm (will select version based on .nvmrc)
$ nvm use
$ nvm install-latest-npm

# 4. Install dependencies & verify latest version is installed (v1.15.0)
$ npm install
$ npm ls eslint-plugin-mdx
[email protected]
└── [email protected]

# 5. Run lint command
$ npm run lint

# 6. Roll-back version of eslint-plugin-mdx
$ npm i -D [email protected]

# 7. Repeat steps 5 & 6 incrementally for v1.13.0, v1.12.0, v1.11.0 and see behavior below.

Expected behaviour

What should happen? eslint should search through the entire project directory for the specified file extensions and run the linting rules against the specific file type. It should parse markdown files with the mdx plugin and identify flaws. The other files will be parsed by their code specific parsers. The mdx plugin should also evaluate the code segments within a markdown file and apply code style rules to those blocks as well. This should not take a long time for the amount of files I have. The CI Pipeline results returned in 7sec and even less on my desktop machine in v1.11.0

Valid CI pipeline results are available for review at https://github.com/codejedi365/treemap-js/runs/3407176892?check_suite_focus=true.

Actual behaviour

In v1.14.0 & v1.15.0:

$ npm exec eslint . --ext ts,js,json,md,mdx,.remarkrc

Oops! Something went wrong! :(

ESLint: 7.32.0

TypeError: Cannot read property 'indent' of undefined
    at adjustMessage (/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/node_modules/eslint-plugin-markdown/lib/processor.js:333:65)
    at Array.map (<anonymous>)
    at /Users/codejedi365/Development/workbench/workspace/TreeMapUtil/node_modules/eslint-plugin-markdown/lib/processor.js:390:22
    at Array.map (<anonymous>)
    at Object.postprocess (/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/node_modules/eslint-plugin-markdown/lib/processor.js:387:34)
    at postprocess (/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/node_modules/eslint-plugin-mdx/lib/processors/remark.js:28:61)
    at Linter._verifyWithProcessor (/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/node_modules/eslint/lib/linter/linter.js:1339:16)
    at Linter._verifyWithConfigArray (/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/node_modules/eslint/lib/linter/linter.js:1273:25)
    at Linter.verify (/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/node_modules/eslint/lib/linter/linter.js:1235:25)
    at Linter.ESLinter.verify (/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/node_modules/eslint-plugin-mdx/lib/processors/options.js:33:19)

In v1.13.0, it never returns >2min.

$ npm exec eslint . --ext ts,js,json,md,mdx,.remarkrc

^C

In v1.12.0: It resolves but it will print out all the issues, including the summary, and then just hang for 30 seconds without doing anything.

$ npm exec eslint . --ext ts,js,json,md,mdx,.remarkrc

.../README.md
  35:25  error  Unable to resolve path to module 'treemap'
                                                                                                                                             import/no-unresolved
  35:34  error  Insert `;`
                                                                                                                                             prettier/prettier
   0:0   error  `processSync` finished async. Use `process` instead
                                                                                                                                             null-null
  30:1   error  Insert `⏎`
                                                                                                                                             prettier/prettier
  35:34  error  Insert `;`
                                                                                                                                             prettier/prettier

.../package.json
  1:1  error  JSON is not sorted  JSON sorting

/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/CHANGELOG.md
  0:0  error  `processSync` finished async. Use `process` instead
                                                                                                                                             null-null

/Users/codejedi365/Development/workbench/workspace/TreeMapUtil/CONTRIBUTING.md
   0:0   error  `processSync` finished async. Use `process` instead
                                                                                                                                             null-null
   1:1   error  Delete `⏎`
                                                                                                                                             prettier/prettier
   7:20  error  Delete `·`
                                                                                                                                             prettier/prettier
   8:44  error  Delete `·`
                                                                                                                                             prettier/prettier
   9:68  error  Replace `·This·saves·` with `This·saves`
                                                                                                                                            prettier/prettier
  45:79  error  Replace `⏎I·` with `·I⏎`
                                                                                                                                             prettier/prettier
  51:80  error  Replace `·actions·in·order·to·evaluate·and·enforce·the·project·standards.··` with `⏎actions·in·order·to·evaluate·and·enforce·the·project·standards.·`
                                                                                                                                             prettier/prettier
  52:48  error  Delete `·`
                                                                                                                                             prettier/prettier
  53:30  error  Delete `·`
                                                                                                                                             prettier/prettier
  54:64  error  Replace `·Please·do·your·due⏎diligence.··` with `Please·do·your⏎due·diligence.`
                                                                                                                                             prettier/prettier

✖ 56 problems (55 errors, 1 warning)
  47 errors and 0 warnings potentially fixable with the `--fix` option.


$

In v1.11.0, it runs with the best performance at about 7 seconds for my project. My CI pipeline results are available for review at https://github.com/codejedi365/treemap-js/runs/3407176892?check_suite_focus=true.

@codejedi365 codejedi365 added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Aug 25, 2021
@JounQin
Copy link
Member

JounQin commented Aug 25, 2021

Please try to use "plugin:mdx/recommended" and its settings in overrides firstly .

You're running mdx/remark processor for all files.

See https://github.com/mdx-js/eslint-mdx#notice

@JounQin
Copy link
Member

JounQin commented Aug 25, 2021

diff --git a/.eslintrc.js b/.eslintrc.js
index e23d28c..826d5b5 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -5,16 +5,7 @@ module.exports = {
     es2017: true,
     node: true
   },
-  extends: [
-    // JS, MD, MDX
-    "eslint:recommended",
-    "airbnb-base",
-    "plugin:mdx/recommended",
-    "plugin:prettier/recommended"
-  ],
-  settings: {
-    "mdx/code-blocks": true
-  },
+  extends: ["eslint:recommended", "airbnb-base", "plugin:prettier/recommended"],
   ignorePatterns: [
     "**/node_modules",
     "dist/**",
@@ -28,6 +19,13 @@ module.exports = {
     "import/no-extraneous-dependencies": "off"
   },
   overrides: [
+    {
+      files: ["*.md", "*.mdx"],
+      extends: ["plugin:mdx/recommended"],
+      settings: {
+        "mdx/code-blocks": true
+      }
+    },
     {
       files: ["*.json", ".remarkrc"],
       plugins: ["json-format"]

@JounQin JounQin closed this as completed Aug 25, 2021
@JounQin JounQin added 🙋 no/question This does not need any changes and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Aug 25, 2021
@JounQin
Copy link
Member

JounQin commented Aug 25, 2021

image

@codejedi365
Copy link
Author

Thank you for the quick reply & fix, that worked significantly. Down to <10 second runtime with the latest version v1.15.

@codejedi365
Copy link
Author

Reopen if necessary...

I spoke too soon. I still have a remark failure TypeError: Cannot read property 'indent' of undefined. What can you tell me about configuring/overriding certain code-block rules and the parser for Typescript code-blocks in markdown?

Failing CI run with TS codeblocks in README.md
https://github.com/codejedi365/treemap-js/runs/3432144040?check_suite_focus=true

If I change them to JS code blocks, then it resolves but then doesn't like the Typescript syntax which can be seen here:
https://github.com/codejedi365/treemap-js/runs/3432123950?check_suite_focus=true

@JounQin
Copy link
Member

JounQin commented Aug 26, 2021

@codejedi365

Then it's a bug of eslint-plugin-markdown.

image

@JounQin
Copy link
Member

JounQin commented Aug 26, 2021

After debugging, message.line is undefined.

{
  "ruleId": null,
  "fatal": true,
  "severity": 2,
  "message":
    "Parsing error: \"parserOptions.project\" has been set for @typescript-eslint/parser.\n" +
    "The file does not match your project config: README.md/0_0.ts.\n" +
    "The file must be included in at least one of the projects provided.",
  "line": undefined,
  "column": undefined
}

@JounQin
Copy link
Member

JounQin commented Aug 26, 2021

See eslint/markdown#191

@JounQin
Copy link
Member

JounQin commented Aug 27, 2021

@codejedi365
Copy link
Author

codejedi365 commented Aug 27, 2021

@JounQin, thank you for continuing to investigate a solution. However,

@codejedi365 Adding excludedFiles: [".md/.ts", ".mdx/.ts"] after https://github.com/codejedi365/treemap-js/blob/main/.eslintrc.js#L32 should resolve your problem.

resolves the undefined error by omitting it to be parsed as Typescript. It then uses the basic JS parser which doesn't throw an exception but rather fails to parse Typescript specific syntax where it is located. Since TS is so closely related to JS, it works on the common syntax. Specifically, the < in fn<type>() does not parse in vanilla JS.

I am pursuing an implementation of a Typescript specific config just for eslint which falls in line with this StackOverflow Post but I haven't gotten it to work yet. I'm not sure how you debugged eslint-plugin-markdown, but the message object post above with the error message hinted to me that its a tsconfig.json include error (of the @typescript-eslint/parser) that is causing the line property to be undefined.

@JounQin
Copy link
Member

JounQin commented Aug 27, 2021

*.mdx/*.ts means virtual filename, and you can't use some type related ts rules, please read its configuration carefully.

https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/TYPED_LINTING.md

@JounQin
Copy link
Member

JounQin commented Aug 27, 2021

@codejedi365
Copy link
Author

eslint/markdown#191

Glad to see this resolution opened since it would of been helpful to see the error from @typescript-eslint/parser for my debugging of the configuration.

Separately, I looked into these links/recommendations and found a better solution:

@codejedi365 Adding excludedFiles: ["*.md/*.ts", "*.mdx/*.ts"] after https://github.com/codejedi365/treemap-js/blob/main/.eslintrc.js#L32 should resolve your problem.

More details: https://eslint.org/docs/user-guide/configuring/configuration-files#how-do-overrides-work

There were 2 parts to my solution:

  1. excludedFiles was necessary to differentiate the regular TS files from the virtual filenames for markdown-code-blocks.
  2. Create a separate JS & TS configs for the markdown code-block virtual filenames. In the TS config block ignore specific TS rules that actually require tsconfig.json and the local files to exist (see response in typescript-eslint/typescript-eslint#3174). There is only 4 rules that have issues and you can't enable the strict type checking ruleset either.

Overall, this solution enabled codeblocks with TS syntax to be parsed correctly (which the default eslint JS parser cannot do) and apply most of all the other Typescript specific rules to the code contained in markdown-code-blocks. My final configuration looks like:

diff --git a/.eslintrc.js b/.eslintrc.js
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ +6,22 +7,23 @@ module.exports = {
    plugins: ["json-format"]
 },
 {
      files: ["*.ts"],
+     excludedFiles: ["*.m{d,dx}/*.ts"],
      parser: "@typescript-eslint/parser",
      parserOptions: {
        ecmaVersion: 8,
        project: "tsconfig.json"
      },
      plugins: ["@typescript-eslint", "jest", "import"],
      extends: [
        "eslint:recommended",
        "airbnb-typescript/base",
        "plugin:@typescript-eslint/recommended",
        "plugin:@typescript-eslint/recommended-requiring-type-checking",
        "plugin:import/recommended",
        "plugin:import/typescript",
        "plugin:jest/recommended",
        "plugin:jest/style",
        "plugin:prettier/recommended"
      ],
@@ -80,14 +100,46 @@ {
      files: ["*.md", "*.mdx"],
      extends: ["plugin:mdx/recommended"],
      settings: {
        "mdx/code-blocks": true
      }
    },
    {
+     // Markdown JS code-blocks (virtual filepath)
+     files: ["**/*.md/*.{js,jsx}"],
+     rules: {
+       // Invalid rules for embedded code-blocks
+       "import/no-unresolved": "off",
+       "no-undef": "off",
+       "no-unused-expressions": "off",
+       "no-unused-vars": "off",
+       "no-unreachable": "off"
+     }
+   },
+   {
+     // Markdown TS code-blocks (virtual filepath)
+     files: ["**/*.md/*.{ts,tsx}"],
+     parser: "@typescript-eslint/parser",
-     parserOptions: {
-       ecmaVersion: 8,
-       project: "tsconfig.eslint.json"
-     },
+     plugins: ["@typescript-eslint"],
+     extends: [
+       "eslint:recommended",
+       "airbnb-typescript/base",
+       "plugin:@typescript-eslint/recommended",
+       "plugin:prettier/recommended"
+     ],
+     rules: {
+       // Additional embedded code-block invalid rules
+       "import/no-unresolved": "off",
+       "no-undef": "off",
+       "no-unused-expressions": "off",
+       "no-unused-vars": "off",
+       "no-unreachable": "off"
+       "@typescript-eslint/no-unused-vars": "off",
+       // Typescript rules that require a project tsconfig.json which is not possible
+       "@typescript-eslint/dot-notation": "off",
+       "@typescript-eslint/no-implied-eval": "off",
+       "@typescript-eslint/no-throw-literal": "off",
+       "@typescript-eslint/return-await": "off",
+       // Readmes should be extra specific or generic as desired
+       "@typescript-eslint/no-inferrable-types": "off",
+       "@typescript-eslint/ban-types": "warn"
+     }
    }
  ]
};

Proof of resolution: https://github.com/codejedi365/avl-treemap/runs/3448583865?check_suite_focus=true

@JounQin
Copy link
Member

JounQin commented Aug 30, 2021

Yes, you're separating code blocks and normal physical .ts files correctly now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 no/question This does not need any changes
Development

No branches or pull requests

2 participants