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

Non JS index files are skipped #128

Closed
OliverJAsh opened this issue Sep 26, 2017 · 20 comments · Fixed by #193
Closed

Non JS index files are skipped #128

OliverJAsh opened this issue Sep 26, 2017 · 20 comments · Fixed by #193

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Sep 26, 2017

Given:

// app.js
import helpers from './helpers'
// helpers/index.ts
export default {}

When I run:

madge --extensions js,ts --warning app.js

I get:

Processed 1 file (587ms) (1 warning)

app.js

✖ Skipped 1 file

./helpers

This seems to be because the index file has a .ts extension instead of .js, however I believe this should be supported.

What do you think?

@mrjoelkemp
Copy link
Contributor

Hey @OliverJAsh! Thanks for filing the issue.

The mixed extension case isn't supported at the moment. The extension of the dependent module (.js) dictates the type of dependency sniffing and partial resolution.

If you changed app.js to app.ts, does madge work fine?

@OliverJAsh
Copy link
Contributor Author

@mrjoelkemp Thanks for the fast response!

It seems not:

~/Development/temp/madge master*
❯ mv app.js app.ts

~/Development/temp/madge master*
❯ madge --extensions js,ts --warning app.ts
Processed 1 file (689ms) (1 warning)

app.ts

✖ Skipped 1 file

./helpers

@OliverJAsh
Copy link
Contributor Author

The mixed extension case isn't supported at the moment.

It might be worth tracking this under a different issue. It would be great to support this—TS doesn't mind if you mix JS and TS imports.

@mrjoelkemp
Copy link
Contributor

@OliverJAsh mind pasting in the debug info? It might require running madge with a debug option; the readme should have specifics.

Feel free to create a new issue for mixed extension support.

@OliverJAsh
Copy link
Contributor Author

@mrjoelkemp How does this look?

❯ madge --extensions js,ts --warning app.ts --debug
⠋ Finding files  madge using src paths [ '/Users/OliverJAsh/Development/temp/madge/app.ts' ] +0ms
  madge using config { baseDir: null, excludeRegExp: false, fileExtensions: [ 'js', 'ts' ], includeNpm: false, requireConfig: null, webpackConfig: null, layout: 'dot', fontName: 'Arial', fontSize: '14px', backgroundColor: '#000000', nodeColor: '#c6c5fe', noDependencyColor: '#cfffac', cyclicNodeColor: '#ff6c60', edgeColor: '#757575', graphVizOptions: false, graphVizPath: false, dependencyFilter: [Function], extensions: 'js,ts', warning: 'app.ts', debug: true } +2ms
  madge using base directory /Users/OliverJAsh/Development/temp/madge +5ms
  tree given filename: /Users/OliverJAsh/Development/temp/madge/app.ts +0ms
  tree resolved filename: /Users/OliverJAsh/Development/temp/madge/app.ts +1ms
  tree visited:  {} +0ms
  tree traversing /Users/OliverJAsh/Development/temp/madge/app.ts +0ms
  precinct options given:  { includeCore: false, type: 'ts' } +0ms
  precinct module type:  ts +1ms
  tree extracted 1 dependencies:  [ './helpers' ] +33ms
  cabinet found a resolver for .ts +0ms
  cabinet performing a typescript lookup +1ms
  cabinet with options:  { module: 2 } +0ms
  cabinet ts resolved module:  undefined +2ms
  cabinet result:  +0ms
  cabinet resolved path for ./helpers:  +0ms
  tree skipping an empty filepath resolution for partial: ./helpers +3ms
  tree cabinet-resolved all dependencies:  [] +0ms
  tree using filter function to filter out dependencies +0ms
  tree unfiltered number of dependencies: 0 +0ms
  tree filtered number of dependencies: 0 +0ms
  tree traversal complete {} +0ms
  tree deduped list of nonExistent partials:  [ './helpers' ] +1ms
  tree object form of results requested +0ms
  tree final tree { '/Users/OliverJAsh/Development/temp/madge/app.ts': {} } +0ms
Processed 1 file (809ms) (1 warning)

app.ts

✖ Skipped 1 file

./helpers

@mrjoelkemp
Copy link
Contributor

Thanks for that. Would you mind double checking that helpers.ts exists in the same folder as app.ts? Just want to rule out some of the easy stuff.

If helpers.ts does exist, then there's an issue with the typescript resolver in filing-cabinet. I must be missing some configuration for the typescript compiler: https://github.com/dependents/node-filing-cabinet/blob/master/index.js#L148.

If you had time, a PR with a failing test case against the filing-cabinet repo would be incredibly helpful. I don't have a lot of time for open source these days, so any help is appreciated.

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Sep 26, 2017

helpers.ts exists in the same folder as app.ts

The helpers module exists at helpers/index.ts, not helpers.ts:

❯ tree one
one
├── app.ts
└── helpers
    └── index.ts

Does this change your diagnosis at all? If not, I will go ahead and investigate the issue against filing-cabinet.

@Kagami
Copy link

Kagami commented Apr 8, 2018

Have same issue.

@marcos-diaz
Copy link

Why is this closed?

I know that this is a bug in node-filling-cabinet, but it makes Madge completely broken for Typescript users with index.ts files, since no dependencies cascade from there.

Specially critical case if someone is relying in Madge as circular dependency detector as part of their CI, and Madge let this pass silently.

@labarilem
Copy link

@marcos-diaz you can pass the --ts-config argument to solve the issue.
E.g.
npx madge app.ts --ts-config=tsconfig.json

@PatrickSchuster
Copy link

I have the same issue. The imported .ts files are ignore eventhough I set up a corresponding tsconfig.json file and added its path to the .madgerc file. Using the --ts.config=tsconfig.json option does not work either.
If I rename the imported .ts file to a .js file, it works as expected. But I would like to avoid having to rename all .ts files.

@adjerbetian
Copy link

I managed to make it work with --ts-config=tsconfig.json, but it doesn't work with the extends option. So you need to provide the tsconfig that has the value compilerOptions.module specified.

In my case, with the structure

packages/
    my-package/
        src/
        tsconfig.json
tsconfig.json

The working command is:

packages/my-package$ madge src/ --extensions ts --ts-config=../../tsconfig.json --image graph.svg

@jonlepage
Copy link

jonlepage commented Dec 22, 2020

Mixed work for me thanks
image

make a tsc init
and pass options { tsConfig: '../tsconfig.json', }

@JackieAnxis
Copy link

Mixed work for me thanks
image

make a tsc init
and pass options { tsConfig: '../tsconfig.json', }

It helps! Thanks a lot.

@blixit
Copy link

blixit commented Jun 27, 2021

I managed to make it work with --ts-config=tsconfig.json, but it doesn't work with the extends option. So you need to provide the tsconfig that has the value compilerOptions.module specified.

thanks @adjerbetian !! I'm using packages and you just saved me hours of debugging. I confirm that extends is not working using madge@5.0.1.

Guys you should consider adding this information to the Readme FAQ

@fabifors
Copy link

@marcos-diaz you can pass the --ts-config argument to solve the issue. E.g. npx madge app.ts --ts-config=tsconfig.json

THANK YOU SOO MUCH. I've been working in the dark since we have a lot of aliases defined in the tsconfig. This addition solved it and now I see the circular dependencies that I knew existed. And now I can see what solves them.

Just want to share my appreciation for this comment that helped me. Thank you! 🙏

@gabriellet
Copy link

I mentioned this on #322, but I wanted to share here as well in case it helps someone else. I 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.

@markkorput
Copy link

markkorput commented Jan 29, 2024

I'm still having this problem with version 6.1.0.

Not a single index.ts file is showing up as a dependency (unless the import line explicitly refers to the index file).

I've explicitly pointed to the tsconfig (both using --ts-config=tsconfig.json and by setting tsConfig in .madgerc

Currently invoking:
madge ./src --ts-config=tsconfig.json

With the following .madgerc:

{
  "fileExtensions": [
    "ts",
    "tsx"
  ],
  "excludeRegExp": [
    ".*styled-system/.*"
  ]
}

@fcsonline
Copy link

@gabriellet & @markkorput, we have been facing the same problems. We did a short investigation, and it looks like the problem is much easier to solve. Check this out:

#402

@egorovsa
Copy link

I'm still having this problem with version 6.1.0.

Not a single index.ts file is showing up as a dependency (unless the import line explicitly refers to the index file).

I've explicitly pointed to the tsconfig (both using --ts-config=tsconfig.json and by setting tsConfig in .madgerc

Currently invoking: madge ./src --ts-config=tsconfig.json

With the following .madgerc:

{
  "fileExtensions": [
    "ts",
    "tsx"
  ],
  "excludeRegExp": [
    ".*styled-system/.*"
  ]
}

Confirm this.. the same . does not work if only I set direct import right to index.ts
in 5.0.1 it was working ok. 6.1.0 is not.
any idea?

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

Successfully merging a pull request may close this issue.