no-cycle reports circular dependencies, but doesn't say what they are

[wolfgang42]

Here's some output I get with "import/no-cycle": ["error", { ignoreExternal: true }]:

$ eslint .

./app.ts
  29:1  error  Dependency cycle detected  import/no-cycle
  32:1  error  Dependency cycle detected  import/no-cycle
  35:1  error  Dependency cycle detected  import/no-cycle

./src/home.ts
  4:1  error  Dependency cycle detected  import/no-cycle

./src/html/render.ts
  4:1  error  Dependency cycle detected  import/no-cycle

./src/ledger/reports.ts
  7:1  error  Dependency cycle detected  import/no-cycle

./src/ui_records.ts
  4:1  error  Dependency cycle detected  import/no-cycle

✖ 7 problems (7 errors, 0 warnings)

As you can see, it's complaining about dependency cycles (probably legitimate ones), but not giving any details on which things depend on what, which makes it difficult to fix them. I know it sometimes reports what the cycles are, at least, because I've seen it output that for other files. But I cleared out all of the ones it explained and I'm still left with some unexplained cycles.

Any idea what might be going wrong here/how I can diagnose? (I can try making a minimal example if needed but this codebase is kind of complicated so I'd like to not spend that time if there's another way to figure out the problem.)

[ljharb]

What’s the line it’s complaining about?

[wolfgang42]

You know, for some reason I hadn't thought to check the line numbers.

These ones are straightforwardly circular:

./app.ts
  29:1  error  Dependency cycle detected  import/no-cycle
  32:1  error  Dependency cycle detected  import/no-cycle
  35:1  error  Dependency cycle detected  import/no-cycle

./src/home.ts
  4:1  error  Dependency cycle detected  import/no-cycle

./src/ledger/reports.ts
  7:1  error  Dependency cycle detected  import/no-cycle

./src/ui_records.ts
  4:1  error  Dependency cycle detected  import/no-cycle

But this one is a bit more confusing:

./src/html/render.ts
  4:1  error  Dependency cycle detected  import/no-cycle

It's complaining about this line (./src/html/render.ts:4):

export {render_editform} from './render/render_editform'

That file, in turn, has this line in it (./src/html/render/render_editform.ts:2):

import {render_form} from '../../html/render'

which is not being flagged by the rule at all despite being a circular dependency (admittedly a weirdly written one, it should probably just be ../render, though fixing that doesn't seem to make a difference).

So I now understand what the circular dependencies are, and more or less why, but I'm still confused about why exactly the error messages are the way they are.

[ljharb]

It’s fair that both should be marked as a cycle, but as long as at least one of them is, you should be able to fix things by removing the cycle.

[wolfgang42]

So this is confusing. I fixed the cycle with app.ts et al, and for some reason removing that cycle made the linter start detecting this apparently unrelated cycle:

./recordtypes/baserecord.ts
  5:1  error  Dependency cycle detected  import/no-cycle

./src/record.ts
  3:1  error  Dependency cycle detected  import/no-cycle

even though I didn't touch anything related to either of those files. It definitely seems like there's something interfering with proper detection here, I think the case of the half-found cycle of ./src/html/render.ts is only a symptom of the problem.

[ljharb]

what are those lines of code in those files?

[wolfgang42]

Just the imports that are, in fact, circular:

import {GenericRecord} from "../src/record"

import {IS_BASERECORD} from '../recordtypes/baserecord'

[ljharb]

ah, so the report is correct, but the bug is that it wasn't reporting it before.

This kind of thing is kind of an NP-hard thing - it's not likely possible to get a complete map, so it's reasonable to stop on the first cycle found.

[wolfgang42]

Yeah, something like that.

For whatever it’s worth, I spent some time messing around with madge --circular and it caught all of these dependencies straight away and explained their cycles immediately—which is sort of a constructive proof that it's possible to get everything at once, at least for my program, though I don't know what sort of problems might be encountered applying that practice to eslint-plugin-import.

[ljharb]

I would be more than happy to review a PR that uses Madge, or its techniques.

[wolfgang42]

I just switched to running it separately (since I don't need this check to be part of eslint specifically anyway), but I'll leave this open for anyone else who runs into the same problems.