pahen / madge

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

False positives when folder name equals node_module name #306

Open Eazymov opened 2 years ago

Eazymov commented 2 years ago

Hi!

Madge version: 5.0.1

Steps to reproduce: 1) Create a project with this very simple structure: src/ --react/ ----a.js ----index.js 2) Fill in the files

// src/react/a.js
import * as React from "react";
// src/react/index.js
import { a } from "./a";

3) Run madge src --circular Error: image

Adding "react" dependency doesn't change anything

lobsterkatie commented 2 years ago

I am also running into this, though in my case my structure looks like this:

src/
  index.server.ts
  config
    webpack.ts
    types.ts
// index.server.ts
export { SomeOtherType } from "./config/webpack";

// config/webpack.ts
import { SomeOtherType } from "./types";

// config/types.ts
import { SomeType } from 'webpack';
export type SomeOtherType = Record<string, any>;

and I am getting

⠋ Finding files⠙ ./index.server.ts⠹ config/types.ts⠸ config/webpack.ts Found 1 circular dependency!

in our Madge GitHub Action.

Interestingly, it was adding the export to index.server.ts which triggered the error. The rest has always passed just fine.(In case it helps, here's the PR: https://github.com/getsentry/sentry-javascript/pull/4597)

EDIT: When I run madge locally, it only lists the last two files (not index.server.js, even though changing it is what triggered the problem).

image

Running it with --debug shows the problem:

image

The second-to-last line shouldn't be src/config/webpack.ts, it should be node_modules/webpack/lib/webpack.js. IOW, madge isn't differentiating between import { thing } from 'stuff' and import { thing } from './stuff'.

wolfgang42 commented 2 years ago

Seems to happen with files, too, not just folders; I have a knex.ts:

import {knex} from 'knex'
export default knex(require('./knexfile'))

... and madge --circular reports it as a dep of itself:

✖ Found 1 circular dependency!

1) knex.ts
lobsterkatie commented 2 years ago

Seems to happen with files, too, not just folders ... and madge --circular reports it as a dep of itself

I just ran into this, too, having forgotten that this was something I needed to worry about. Any progress here? Can we do anything to help move this fix along? (UPDATE: Just found https://github.com/pahen/madge/issues/334. TL;DR - Life happens, so we probably won't see any movement here until early next year.)