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

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

Open
eelco opened this issue Aug 29, 2022 · 11 comments
Open

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

eelco opened this issue Aug 29, 2022 · 11 comments
Labels

Comments

@eelco
Copy link

eelco commented Aug 29, 2022

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 eelco changed the title Circular imports as missed when using …/index.ts files Circular imports are missed when using …/index.ts files Aug 29, 2022
@eelco
Copy link
Author

eelco commented Aug 29, 2022

I dug into this and found the underlying bug: dependents/node-filing-cabinet#102

@eelco
Copy link
Author

eelco commented Aug 29, 2022

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
Copy link
Author

eelco commented Aug 29, 2022

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

  • filing-cabinet has an odd default, that does not match the TypeScript default
  • madge is not automatically picking up the tsconfig.json file (I somehow expected this)

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
Copy link
Collaborator

looks similar to

When fixing this, try fix the other one together.

@divisnotabutton
Copy link

divisnotabutton commented Feb 24, 2023

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
Copy link

vbraun commented Apr 14, 2023

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
Copy link

vbraun commented Apr 14, 2023

This is probably the same reason as #271 (comment), 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
Copy link

vbraun commented Apr 14, 2023

EDIT: never mind, forgot that I had edited filing-cabinet. The issue is the same as #271 (comment)

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
Copy link

pokey commented Nov 22, 2023

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
Copy link

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
Copy link

I ran into this too on madge 6.1.0. Seems like an update to dependency-tree will fix it. #371 (comment)

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

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

No branches or pull requests

7 participants