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

Version 1.0 #96

Merged
merged 39 commits into from
Aug 19, 2016
Merged

Version 1.0 #96

merged 39 commits into from
Aug 19, 2016

Conversation

pahen
Copy link
Owner

@pahen pahen commented Aug 16, 2016

This is what's going to be Madge 1.0. See changelog and updated documentation.

Want to be a beta tester? Give it a spin and install it like this:

npm install -g git://github.com/pahen/madge.git#v1

Would love to get some feedback!

Fixes #88
Fixes #68
Fixes #57
Fixes #56
Fixes #52

@pahen
Copy link
Owner Author

pahen commented Aug 16, 2016

Wanna give it a try @mrjoelkemp @Kriegslustig ? :)

@mrjoelkemp
Copy link
Contributor

I can give it a spin on a few repos tonight. Excited to see this land!

@pahen
Copy link
Owner Author

pahen commented Aug 16, 2016

That would be awesome @mrjoelkemp. I'm very excited to see this land too!

@pahen pahen changed the title [wip] Version 1.0 [WIP] Version 1.0 Aug 16, 2016
@mrjoelkemp
Copy link
Contributor

I tried running DEBUG=* time madge . --image graph.svg within a local clone of eslint and found things to take a really long time (many minutes and still didn't even stop).

What's a bit puzzling is that there are files within node_modules that are in visited – despite the filter function being applied. Ideally, the files below should not be in the visited tree.

image

You may need to filter out node_modules during this directory tree walk before calling dependency-tree. Dependency-tree does apply the filter function (which is why all of the 3rd party modules have no children), but it still has to power through the many many many npm modules that are within node_modules.

@pahen
Copy link
Owner Author

pahen commented Aug 17, 2016

Thanks! Good point! I'll take a look at that :)

@pahen
Copy link
Owner Author

pahen commented Aug 17, 2016

Should be fixed now @mrjoelkemp! But avoid running it in DEBUG mode since it will be much slower. It took 22s for me now to run time madge . on the eslintproject.

@mrjoelkemp
Copy link
Contributor

Nice job with the fix! Looks good to me. Tried it on my ES6 work project and it looks great. Looking forward to seeing the errors that users report. I'll be sure to prioritize any dependency-tree bugs as they come up.

@frederikschubert
Copy link

frederikschubert commented Aug 18, 2016

I tried using it on a typescript project but it only works on some files. On Some files I get:

precinct could not parse content +15ms

Will try to debug it further and report back.

Update

It seems that precinct cannot handle decorators.
Babylon cannot handle the keywords public, private or protected. Decorators are not the problem.
If I add the following lines to precinct it works:

content = content.replace(/private\s/, '').replace(/public\s/, '').replace(/protected\s/, '');

@pahen
Copy link
Owner Author

pahen commented Aug 18, 2016

How do you execute madge? It would be nice to see the parse error in precinct @mrjoelkemp. Maybe change to something like debug('could not parse content: %s', e.message) instead?

@frederikschubert
Copy link

frederikschubert commented Aug 18, 2016

madge --debug src/entities/sender/sender.api.ts    

I applied your change and now I get:

precinct could not parse content: Unexpected token (23:11) +15ms

@mrjoelkemp
Copy link
Contributor

The change could be made but any parse errors (syntax errors or unknown tokens) trigger it, so the error message is implied. I'd gladly merge a PR that adds more helpful logging though.

To my knowledge, Typescript isn't supported via babylon (dependency-tree's parser). This is another case for detective-typescript. We'd need an associated partial resolver registered in filing-cabinet. I don't have capacity to take this on at the moment unless there is a large demand. I'll happily review prs though.

@pahen
Copy link
Owner Author

pahen commented Aug 18, 2016

Thanks for reporting @frederikschubert! I'm moving TypeScript support (#64) into the 1.1.0 milestone instead of this release.

@pahen pahen changed the title [WIP] Version 1.0 Version 1.0 Aug 19, 2016
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 this pull request may close these issues.

3 participants