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

Support TypeScript #64

Closed
alexeagle opened this issue Oct 2, 2015 · 24 comments
Closed

Support TypeScript #64

alexeagle opened this issue Oct 2, 2015 · 24 comments

Comments

@alexeagle
Copy link

We use madge to find circular deps in Angular. We had been outputting to ES6, which lets us do a symbol-level graph (instead of file-level like commonjs)
But we no longer use the ES6 output for anything else, so I'd like to remove it.

I tried pointing madge at our sources

 {
    format: 'es6',
    paths: ['modules/angular2'],
    extensions: ['.ts']
  }

but I get an empty set of deps for each module, so I think node-detective-es6 or node-source-walk are getting confused parsing the TS files (even though we use ES6 module syntax).

@louy
Copy link

louy commented Dec 18, 2015

👍 I'd love that

@jkillian
Copy link

jkillian commented Jan 4, 2016

👍 from me as well

@ulfryk
Copy link

ulfryk commented Jun 15, 2016

👍

@pahen
Copy link
Owner

pahen commented Jul 4, 2016

Do you think this should be supported in detective-es6 @mrjoelkemp ?

@mrjoelkemp
Copy link
Contributor

It comes down to what the parser understands. Detective-es6 is powered by babel's parser. If that can support typescript, then it's only a matter of seeing which ast node type is used for its imports.

My hunch, though, is that there's room for a new detective-typescript that's powered by the typescript parser.

Happy to support typescript in node-source-walk if that's configurable through babylon (babel's parser).

@pahen
Copy link
Owner

pahen commented Jul 4, 2016

Putting this into detective-typescript sounds like a good plan! :)

@pahen pahen self-assigned this Aug 5, 2016
@pahen pahen modified the milestone: 1.0 Aug 8, 2016
@pahen pahen mentioned this issue Aug 16, 2016
@pahen pahen modified the milestones: 1.1.0, 1.0 Aug 18, 2016
@pahen pahen removed their assignment Aug 19, 2016
@pahen
Copy link
Owner

pahen commented Aug 19, 2016

See last comments in #96

@pahen pahen closed this as completed Aug 19, 2016
@pahen pahen reopened this Aug 19, 2016
@pahen pahen removed this from the 1.1.0 milestone Aug 23, 2016
@mrjoelkemp
Copy link
Contributor

For this, we need to figure out a plug and play approach for precinct and filing cabinet.

There's a possibility that we could add a static, register method to precinct that attaches new detectives. If that happens within madge, then we could rely on node's module caching to make sure that precinct itself picks up the runtime-attached detectives.

Filing-cabinet already has a register method.

If this runtime attachment doesn't work, we may need to pass configuration all the way down for precinct to register the configured detectives itself. The config would be a file extension to detective mapping. Something similar could happen for the cabinet resolver.

Thoughts?

@pahen
Copy link
Owner

pahen commented Sep 7, 2016

I think it would be best to keep custom detectives outside of madge if possible. So adding the needed logic to precinct itself so it's able to handle TypeScript automatically would be awesome.

@mrjoelkemp
Copy link
Contributor

To clarify, I was thinking that we'd have separate detective-typescript and typescript-lookup modules that madge would include as dependencies and register on precinct and filing-cabinet, respectively.

My long-term goal for dependency-tree is to be a language-agnostic dependency tree generator. I want to support as many languages as possible – without carrying around the "kitchen sink." For example, I don't think it's beneficial for PHP codebases to include all of the JS or SASS language support when they don't need it.

To support typescript, we'd have to bring along the typescript compiler. This doesn't have to live in node-source-walk (as of dependents/node-source-walk#19) thankfully. However, it does mean that the typescript compiler becomes a dependency of detective-typescript and typescript-lookup. If those modules become dependencies of precinct and filing-cabinet, then every consumer of dependency-tree downloads the typescript compiler.

Aside from only downloading what you need, it's a good growth strategy to allow others to create their own language support that plugs into dependency tree without requiring the underlying libs to change.

@pahen
Copy link
Owner

pahen commented Sep 7, 2016

Ok, thanks for clarifying. I think I misunderstood you :) Sounds like a good way forward! 👍

@mrjoelkemp
Copy link
Contributor

I can work on testing out whether or not the runtime-attachment of detectives is even feasible. I'll report back with my findings in a few days.

@pahen did you plan to work on detective-typescript?

@pahen
Copy link
Owner

pahen commented Sep 7, 2016

Sounds great!

Not at the moment. I don't have much free time just now. But maybe later if nobody else start to work on it :)

@scottyantipa
Copy link

TS and others (coffee, jsx) would be great!
Are there any plans to move forward with the detective-* approach?

@pahen
Copy link
Owner

pahen commented Nov 19, 2016

JSX should actually be supported already. Haven't decided about extending it with support for custom detectives yet.

@pahen
Copy link
Owner

pahen commented Jan 17, 2017

Could use https://github.com/eslint/typescript-eslint-parser. ESTree ast structure should already work with dependency tree.

@pahen
Copy link
Owner

pahen commented Jan 23, 2017

I've created detective-typescript now @mrjoelkemp using the typescript-eslint-parser and it works very well so far :) Should we maybe move the repo to https://github.com/dependents instead where all the other detectives are?

The next step is to see how we can use it in a good way with dependency-tree without making it required.

@mrjoelkemp
Copy link
Contributor

Wow @pahen! Awesome work! How big is the eslint typescript parser? A dynamic dependency is complex and I'd prefer to avoid it until the download time for dependency-tree becomes a problem.

We could plug this into precinct pretty easily. We'll need the module name to file resolution in filing cabinet though.

Your module doesnt have to be in the dependents org, but thanks for offering.

As a next step, will you create the resolver for filing cabinet?

@pahen
Copy link
Owner

pahen commented Feb 8, 2017

Thanks @mrjoelkemp! :) The typescript-eslint-parser is 168K unpacked on the disk, so not extremely big. I'll take a look at the resolver for filing cabinet soon and see if I can have it work with precinct and madge :)

@karfau
Copy link

karfau commented Jun 18, 2017

Is there anything that can already be done "by hand" to make madge work with typescript?

I only understand half of what you are discussing, so I might have a hard time trying to help, but I could try.
If you have an idea what specifically you need help with, maybe put a list here so people can pick up stuff?

Best Christian

@pahen
Copy link
Owner

pahen commented Jul 11, 2017

Thanks for offering you help Christian! But typescript support is on hold for now. The easiest way to make it work is to run it on the compiled source.

@gtramontina
Copy link

http://angularjs-graph.org/ can give a lot of insights. It works well for angular 1.x, not sure about angular 2 though. It runs on a running angular application, not as "static analysis" per se.

mrjoelkemp added a commit to dependents/node-filing-cabinet that referenced this issue Aug 26, 2017
mrjoelkemp added a commit to dependents/node-filing-cabinet that referenced this issue Aug 26, 2017
@mrjoelkemp
Copy link
Contributor

Support for Typescript has landed in the v5.10.x release of https://github.com/dependents/node-dependency-tree.

We need to bump the version of that dependency here within madge and then write madge tests to prevent regressions. Any takers?

Big thanks to the following folks:

We're almost at the finish line. 🎊

@pahen
Copy link
Owner

pahen commented Aug 26, 2017

Added in Madge v2.1.0! 🎉 💃

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

No branches or pull requests

9 participants