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

v2 is SLOW #615

Closed
indeyets opened this issue Oct 7, 2016 · 12 comments · Fixed by #654
Closed

v2 is SLOW #615

indeyets opened this issue Oct 7, 2016 · 12 comments · Fixed by #654

Comments

@indeyets
Copy link

indeyets commented Oct 7, 2016

Upgrading eslint-plugin-import led to significant change in speed of eslint scan on several codebases I support.

For example, https://github.com/FreeFeed/freefeed-server:

  • 1.x: 16 seconds
  • 2.x: ~110 seconds

Is this expected?

@masylum
Copy link

masylum commented Oct 7, 2016

Same here. I had to disable it because of the performance regression. Is there a way we can help?

@benmosher
Copy link
Member

Interesting. Could be that the fail-fast regex for unambiguous module detection is getting a lot of false positives.

You could try using the import/ignore setting to ignore node_modules; if it helps considerably, I suspect this is the issue.

@indeyets
Copy link
Author

@benmosher I added the following to my .eslintrc:

  "settings": {
    "import/ignore:": [
      "node_modules"
    ]
  },

it doesn't help

@jfmengels
Copy link
Collaborator

@indeyets Hmm, could you try to deactivate rules to find out which one is causing the lag? @benmosher seems to think it's the unambiguous rule, but it would be helpful to know for sure.

@masylum
Copy link

masylum commented Oct 12, 2016

My configuration is this one if that can be helpful:

{
  "parser": "babel-eslint",
  "plugins": [
    "import",
    "react",
    "flowtype"
  ],
  "extends": [
    "plugin:import/errors",
    "plugin:import/warnings",
    "plugin:react/recommended",
    "plugin:flowtype/recommended",
    "standard",
    "standard-jsx",
    "standard-react"
  ],
  "rules": {
    "object-curly-spacing": ["warn", "always"],
    "flowtype/no-weak-types": [0, {
      "any": false,
      "Object": false,
      "Function": false
    }]
  },
  "settings": {
    "import/resolver": {
      "webpack": {
        "config": "webpack.config.js"
      }
    },
    "import/extensions": [".js", ".jsx"]
  }
}

@benmosher
Copy link
Member

@indeyets:

"import/ignore:": [

Is that extra colon inside the quotes actually in your config?

@benmosher
Copy link
Member

Btw @jfmengels + all: I'm not saying it's the unambiguous rule, I'm saying it's the new auto-ignore that parses dependencies to see if they have exports or imports, and ignores them if they don't.

Historically, most of the time is dependency parse time. There is a regex that is a lot faster than a full JS parse that should prevent parsing most non-ES-module deps. I'm thinking some part of that must not be working for @indeyets, thus my suspicion that explicitly ignoring node_modules would help a lot.

Probably just need to add some timing and debug logging and do some real forensics. 😅 I didn't notice a bug slowdown on my work projects with tons of CJS deps, so maybe those that have commented here have something going on that I don't yet understand. But if it was fast before, it should be able to be made fast again. 😄

@indeyets
Copy link
Author

indeyets commented Oct 17, 2016

@benmosher you're right. the colon was an issue. I fixed the setting and scan-time is reasonable again 👍

@jfmengels
Copy link
Collaborator

Cool!

@benmosher
Copy link
Member

@indeyets sweet! it does mean you're losing any linting of node_modules that have jsnext:main configured, though.

at some point I'd like to profile the linked project without ignoring node_modules and try to identify the issue; it still shouldn't be much different than v1.

@benmosher benmosher added this to the v3 - import/order internal milestone Nov 3, 2016
@benmosher
Copy link
Member

reopening because I want to fix in v3

@benmosher benmosher reopened this Nov 3, 2016
@benmosher benmosher modified the milestones: unambiguous regex false positives, v3 - import/order internal Nov 3, 2016
benmosher added a commit that referenced this issue Nov 5, 2016
@benmosher
Copy link
Member

@indeyets good news, with the fixes in b35d489 your freefeed-server project lints in <10s, without the import/ignore setting. 😅 will publish ASAP!

benmosher added a commit that referenced this issue Nov 7, 2016
* fix logical gaffs preventing unambiguous regex from functioning properly.

fixes #615
fixes #634
fixes #649

* fixed typo 😅
@benmosher benmosher mentioned this issue Jun 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants