pahen / madge

Create graphs from your CommonJS, AMD or ES6 module dependencies
MIT License
8.95k stars 318 forks source link

Circular imports are missed when using `…/index.ts` files #322

Open eelco opened 2 years ago

eelco commented 2 years ago

Madge is not able to identify circular imports when using a folder with an index.ts file.

To Reproduce

aap/index.ts:

export { default as Noot } from "../noot"

noot.ts:

import { mies } from "./mies"

export default class Noot {}

mies.ts:

import { Noot } from "./aap"

export const mies = 42

When renaming aap/index.ts to aap.ts and changing the Noot import to ./noot, it does see the circular import.

eelco commented 2 years ago

I dug into this and found the underlying bug: https://github.com/dependents/node-filing-cabinet/issues/102

eelco commented 2 years ago

Ignore that, this seems to be another issue. When I (explicitly) pass madge the --ts-config option that points to a file with { "compilerOptions": { "module": "esnext", "moduleResolution": "node" } }, it will correctly resolve (because using the index.ts is a node-ism).

eelco commented 2 years ago

So, it seems the bug can be attributed to different things:

I’m still seeing some odd things going on when using it with the tsconfig.json file of our project however (still not finding all circular files), I’ll investigate and file different issues.

I’ll leave this issue open for the maintainers to decide if they want to work around the filing-cabinet behavior or not.

PabloLION commented 1 year ago

looks similar to

When fixing this, try fix the other one together.

divisnotabutton commented 1 year ago

Same issue when using .depends().

Seems to ignore res that imported the module without index file in path import { MyButton } from 'components/my-button. Works fine for res with import { MyButton } from 'components/my-button/index.ts.

With "moduleResolution": "node" it seems to ignore res that were present in output previously (when Classic module resolution strategy was applied).

Is there a way to efficiently debug this? I'm in a monorepo-ish project with complex structure and lots of configurations for various cases (multiple tsconfigs, multiple webpack configs etc).

vbraun commented 1 year ago

I have "moduleResolution": "node", in my tsconfig.json, and madge 5.0.1 used to resolve the index.ts file. But after updating to madge 6.0.0 the index.ts is not checked, and the directory path to the index files is printed among the skipped file warnings.

vbraun commented 1 year ago

This is probably the same reason as https://github.com/pahen/madge/issues/271#issuecomment-1484707370, the parsed integer and not original string for moduleResolution is passed down to filing-cabinet.

$ madge --debug --ts-config tsconfig.json src/main.ts 
[...]
2023-04-14T11:09:34.879Z cabinet given typescript config:  {
  compileOnSave: false,
  compilerOptions: {
    [...]
    module: 1,
    moduleResolution: 2,
vbraun commented 1 year ago

EDIT: never mind, forgot that I had edited filing-cabinet. The issue is the same as https://github.com/pahen/madge/issues/271#issuecomment-1484707370

OLD: OK the relevant setting is module, not moduleResolution. When I use

$ cat tsconfig.madge.json
{
    "extends": "./tsconfig.json",
    "compilerOptions": {
        "module": "commonjs",
    }
}
$ madge --ts-config tsconfig.madge.json src/main.ts 

then the index.ts files are checked.

pokey commented 10 months ago

Strangely, that fix of passing in a tsconfig with module of commonjs no longer seems to work. It works fine on madge version 5.0.1, but not on madge version 6.1.0

gabriellet commented 8 months ago

Also ran into this issue where imports in TypeScript files that use an index file were not detected on madge version 5.0.1. We were already passing the tsconfig.json file to madge. When we removed the extends in our tsconfig.json and also changed compilerOptions.moduleResolution from bundler to node, madge worked as expected and is able to follow the imports from TypeScript files.

kirbysayshi commented 7 months ago

I ran into this too on madge 6.1.0. Seems like an update to dependency-tree will fix it. https://github.com/pahen/madge/issues/371#issuecomment-1965562870

I tried the workarounds listed here (new tsconfig, changing moduleResolution, etc) but only upgrading dependency-tree worked for m.