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

linter: 0.13.0 is hanging with --import-plugin (or taking more than 5 minutes) #7406

Closed
anthonyshew opened this issue Nov 21, 2024 · 13 comments · Fixed by #7432
Closed

linter: 0.13.0 is hanging with --import-plugin (or taking more than 5 minutes) #7406

anthonyshew opened this issue Nov 21, 2024 · 13 comments · Fixed by #7432
Labels
C-bug Category - Bug

Comments

@anthonyshew
Copy link

anthonyshew commented Nov 21, 2024

Was hoping to try out the fix for #7123 but adding --import-plugin is causing Oxlint to hang. I see output for many found errors but then it halts suddenly.

I'm trying to eventually run oxlint -A all -D no-cycle --import-plugin so trying to see what works and what doesn't:

Working:

  • oxlint, exits as expected
  • oxlint -A all, exits as expected
  • oxlint -D no-cycle, exits as expected

Not working:

  • oxlint --import-plugin, produces output for many errors, then halts
  • oxlint -A all --import-plugin, no output, appears to hang (makes sense)
  • oxlint -D no-cycle --import-plugin, produces output for many errors, then halts

This appears to be a regression specifically in 0.13.0. 0.12.0 is working as expected in all cases.

@anthonyshew anthonyshew added the C-bug Category - Bug label Nov 21, 2024
@anthonyshew anthonyshew changed the title 0.13.0 is hanging with --import-plugin (or taking more than 5 minutes) linter: 0.13.0 is hanging with --import-plugin (or taking more than 5 minutes) Nov 21, 2024
@camc314
Copy link
Contributor

camc314 commented Nov 21, 2024

is this a regression? does it work on older versions?

@anthonyshew
Copy link
Author

anthonyshew commented Nov 21, 2024

Yes. 0.12.0 runs as expected for me.

Sorry, forgot to put that into OP. Added it in now.

@camc314
Copy link
Contributor

camc314 commented Nov 21, 2024

got it, thanks. do you have a repo i can repro this on?

@anthonyshew
Copy link
Author

I unfortunately don't. I'm seeing this in our massive monorepo and I wouldn't know where to start with figuring out with deconstructing the matrix of possibilities that could be making this happen.

I was hoping there was something that made sense for this on the diff between 0.12 to 0.13, but I don't know enough about the codebase to know what would make sense. 😄

If there's nothing that sticks out to you, I can maybe try to bisect/cherry-pick between 0.12 and .013, build from source, and see if I can find something?

@Grohden
Copy link

Grohden commented Nov 22, 2024

Seeing the same.. just following the simplest example from the website in our codebase makes it hang for me:

oxlint --config=./oxlintrc.json ./file-path

with

oxlintrc.json (from the site) as

{
  "$schema": "./node_modules/oxlint/configuration_schema.json",
  "plugins": ["import", "typescript", "unicorn"],
  "env": {
    "browser": true
  },
  "globals": {
    "foo": "readonly"
  },
  "settings": {},
  "rules": {
    "eqeqeq": "warn",
    "import/no-cycle": "error"
  },
  "overrides": [
    {
      "files": ["*.test.ts", "*.spec.ts"],
      "rules": {
        "@typescript-eslint/no-explicit-any": "off"
      }
    }
  ]
}

As soon as I remove the 'import' plugin it doesn't hang

@Grohden
Copy link

Grohden commented Nov 22, 2024

Checking here, we have a path in tsconfig configured like this:

{
  "compilerOptions": {
     "paths": {
        "BAR/*": ["*"]
     },
  }
}

If I comment it, or I change it to

{
  "compilerOptions": {
     "paths": {
        "BAR/src/*": ["./src/*"]
     },
  }
}

oxlint doesn't hang (["*"] seems to be the root cause for me)

edit: a simple change from "BAR/src/*": ["/src/*"] to "BAR/src/*": ["./src/*"] causes it to hang 😟 which means I'm misconfiguring paths, and when I do, it works because its probably not checking paths

@camc314
Copy link
Contributor

camc314 commented Nov 22, 2024

i can repro this with https://github.com/jsx-eslint/eslint-plugin-jsx-a11y
and npx oxlint@latest -A all -D no-cycle --import-plugin .

@anthonyshew
Copy link
Author

I can confirm that's in our repository, so that could be my case.

@camc314
Copy link
Contributor

camc314 commented Nov 22, 2024

sorry meant if i clone that repo and run it i can repro this hanging

@camc314
Copy link
Contributor

camc314 commented Nov 22, 2024

it appears to hang here.

let mut state = cvar
.wait_while(lock.lock().expect("Failed lock inner cache state"), |state| {
matches!(*state, CacheStateEntry::PendingStore(_))
})
.unwrap();

@camc314
Copy link
Contributor

camc314 commented Nov 22, 2024

bisecting now,

but i'm guessing it's this commit c00f669 looks like that was in 0.12.0

@camc314
Copy link
Contributor

camc314 commented Nov 22, 2024

ok, so it broke in 878189c

hmm i don't know enough about this to do a fix. @Boshen any ideas?

the reason this is happening is because we early return here: if there're any parser errors OR the lang was flow

return if ret.is_flow_language {
vec![]
} else {
ret.errors.into_iter().map(|err| Message::new(err, None)).collect()
};

however we've already inserted the record here:

if self.init_cache_state(path) {

as we early returned, we never hit this line:

self.modules.add_resolved_module(path, Arc::clone(&module_record));

i guess we could insert an empty module record if we failed to parse/the lang is flow?

@anthonyshew
Copy link
Author

Running for me now! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category - Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants