Ignore "import type" statements for Flow type annotations

[lll000111]

Type systems are completely independent of the Javascript runtime and really don't matter because they don't influence runtime module imports, since they are only used by the statically run type system and even circular dependencies don't matter at all since nothing is executed.

madge interprets "import type" statements as module imports and Javascript module dependencies, but that is not true. I get a completely different result when I let it run on ./src/ compared to my after-built (i.e. all type annotations stripped) ./lib/.

Madge could either ignore "import type" or use "flow-remove-types" (https://github.com/flowtype/flow-remove-types).

Yes I know, the simple workaround is to just run madge on the build JS files instead of the sources. Still, I'm not sure how many people don't think about this and end up with an overly complicated dependency graph that does not reflect the actual module loading dependencies. Maybe a note in the README should mention this. Not sure about TypeScript, it's more complicated, I guess that can only be dependency-parsed after built anyway?

[mrjoelkemp]

Thanks for contributing @lll000111! If the import type syntax is only applicable to es6 modules, then the fix (I think the ast sniff is too weak at the moment, so it's a bug fix in my eyes) can be added here (along with tests) https://github.com/dependents/node-detective-es6/blob/master/index.js#L24.

I'm happy to review a PR with the fix if you have the time to implement it.

[lll000111]

I checked it out but I admit I find it a bit confusing. It says it supports Flow, and there is a test too:

https://github.com/dependents/node-source-walk/blob/master/test/test.js#L259

But that tests some obscure syntax only:

import { Something, type SomethingElse } from "someModule";

The regular type import in Flow is

import type {Foo} from ''/bar.js';

I'm not sure what the tests actually tests either. What is the result? I would expect it to ignore type imports completely. It seems to me that that is not the case, and that the maintainer's intent is different from what I think should happen. So a PR without prior discussion about what the intended/desirable behavior is seems futile?

[mrjoelkemp]

Thanks for investigating @lll000111.

That test is only to ensure that the parser doesn't throw an exception when encountering flow syntax. The bug you're mentioning is separate.

It sounds like you know the buggy behavior and what it should be doing instead.

Im happy to resolve any further confusion that prevents a PR from being proposed.

I don't use flow, so community guidance is appreciated.

[abewinterhq]

Hi! My team is thinking about using madge for circular dependency checking but we need the ability to ignore import type.

I posted a PR to detective-es6 (referenced above) that would support this via .madgerc. More details & background in the PR.

[abewinterhq]

PR landed -- can probably close this

[lll000111]

I checked the latest commits and PRs, which one is it? I don't see it...

Oh I see, link: dependents/node-detective-es6#13

Is it automatically used by madge now?

[abewinterhq]

my guess is yes if you:

1. reinstall so you get the newest dependencies (and make sure they're not pinned in your package-lock)
2. follow .madgerc instructions in the PR

[lll000111]

Well I just assume it works; Closing... :-)