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

Comprehend Sugared AMD Modules #8

Closed
EisenbergEffect opened this issue Dec 20, 2012 · 17 comments
Closed

Comprehend Sugared AMD Modules #8

EisenbergEffect opened this issue Dec 20, 2012 · 17 comments

Comments

@EisenbergEffect
Copy link

With require.js, you can write AMD modules using the sugared syntax:

define(function(require){
var mod1 = require('mod1') ;
});

I've run madge over a project which uses this, but it only seems to detect the top most module dependencies and not the rest.

@pahen
Copy link
Owner

pahen commented Dec 20, 2012

I'm using this style myself in a project and it seems to work pretty well for me. Can you give me a more detailed example so I can try to reproduce it?

@EisenbergEffect
Copy link
Author

Well, then....it's highly likely I just have no idea what I'm doing 😄 Here's a link to the code I was trying to generate a graph of: https://github.com/EisenbergEffect/Durandal/tree/master/app/durandal

Let me know if I can help any other way. Thanks!

@pahen
Copy link
Owner

pahen commented Dec 21, 2012

Hehe, I'm sure you know what you're doing :) I tried running madge on your project and it seems to be working as expected for me. Am I missing something in your code?

$ madge -f amd -i graph.png app/durandal

http://cl.ly/image/1P1K2Y1e2O0x

$ madge -f amd app/durandal
app
  composition
  dom
  events
  messageBox
  modalDialog
  system
  viewEngine
  widget
composition
  system
  viewEngine
  viewLocator
  viewModelBinder
dom
  system
events
  system
http
messageBox
modalDialog
  composition
  system
  viewModel
system
viewEngine
  dom
  text
viewLocator
  system
  viewEngine
viewModel
  system
viewModelBinder
  system
widget
  composition
  system

@EisenbergEffect
Copy link
Author

Well, would you look at that!? It works! I think I must have been using the command incorrectly. I'll give it a try myself and see what happens.

@EisenbergEffect
Copy link
Author

Ok. I tried it on my machine and I get something different:

app
composition
dom
events
messageBox
modalDialog
system
viewEngine
widget

It's like it's only tracing the app module and not the other modules. I'm running this on Windows...could that be related?

@pahen
Copy link
Owner

pahen commented Dec 21, 2012

Hmm, that's weird! I never tried it in windows before .. I'll boot up my virtual machine and give it a try!

@pahen
Copy link
Owner

pahen commented Dec 21, 2012

I tried it now, and confirmed it's a problem when running madge on Windows :( To much modules that I use seems to be incompatible with Windows at this time. I can't even run the unittests. Run it on Mac or Linux and it should be ok.

@pahen pahen closed this as completed Dec 21, 2012
@EisenbergEffect
Copy link
Author

Well, that's unfortunate. Hopefully that can be remedied in the future. For now, the image you generated will actually work for me. When I need a new image...I'll swipe my wife's mac while she's not looking ;) Thanks for looking into this issue and for working on this project!

@israels
Copy link

israels commented Jan 9, 2013

I also was having this issue and traced it to an issue with the dependent library findit: https://github.com/substack/node-findit/issues/5

If you modify the index.js file within the findit library (as mentioned in the link above) you can enable a workaround and get madge file enumeration to work on windows.

findit in turn is using node libraries for file enumeration so a related issue is opened in the node libuv package
joyent/libuv#613

Per comments in the original issue, an alternative file enumerating library may work on windows, but would be up to pahen if he wants to change to it: https://github.com/soldair/node-walkdir

@pahen
Copy link
Owner

pahen commented Jan 10, 2013

Thanks very much @israels for looking into this issue. Since the API of node-walkdir was compatible with node-findit I've now done the switch and it seems to work just fine. Tested it in Windows too, and it works much better now. I published v0.1.4 to NPM with the fix. @EisenbergEffect may be interested to give it a spin again now I suppose :)

@EisenbergEffect
Copy link
Author

Awesome!

On Thu, Jan 10, 2013 at 3:41 PM, Patrik Henningsson <
[email protected]> wrote:

Thanks very much @israels https://github.com/israels for looking into
this issue. Since the API of node-walkdir was compatible with node-findit
I've now done the switch and it seems to work just fine. Tested it in
Windows too, and it works much better now. I published v0.1.4 to NPM with
the fix. @EisenbergEffect https://github.com/EisenbergEffect may be
interested to give it a spin again now I suppose :)


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-12117538.

Rob Eisenberg,
Blue Spire Consulting, Inc.
Caliburn Project
850.264.3996

@EisenbergEffect
Copy link
Author

Running now gives me:

module.js:340
throw err;
^
Error: Cannot find module 'walkdir'
at Function.Module._resolveFilename (module.js:338:15)
at Function.Module._load (module.js:280:25)
at Module.require (module.js:362:17)
at require (module.js:378:17)
at Object. (C:\Users\EisenbergEffect\AppData\Roaming\npm\node_mod
ules\madge\lib\parse\base.js:10:11)
at Module._compile (module.js:449:26)
at Object.Module._extensions..js (module.js:467:10)
at Module.load (module.js:356:32)
at Function.Module._load (module.js:312:12)
at Module.require (module.js:362:17)

If I install the walkdir package manually, it seams to work.

@pahen
Copy link
Owner

pahen commented Feb 3, 2013

That seems very weird since package.json is up to date ..

Can you list the folders in C:\Users\EisenbergEffect\AppData\Roaming\npm\node_mod
ules\madge\node_modules?

Did you install walkdir globally or locally to fix the problem?

@EisenbergEffect
Copy link
Author

Folder there are: coffee-script, colors, commander, commandir, detective,
findit, graphiz, resolve, uglify-js
I fixed the problem with a global install of walkdir.

On Sun, Feb 3, 2013 at 9:49 AM, Patrik Henningsson <[email protected]

wrote:

That seems very weird since package.json is up to date ..

Can you list the folders in
C:\Users\EisenbergEffect\AppData\Roaming\npm\node_mod
ules\madge\node_modules?

Did you install walkdir globally or locally to fix the problem?


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-13047896.

Rob Eisenberg,
Blue Spire Consulting, Inc.
Caliburn Project
850.264.3996

@pahen
Copy link
Owner

pahen commented Feb 3, 2013

Strange .. and you're installing madge with NPM? If so, can you copy the entire output when running "npm install madge" and include it here? If you're installing directly from the GitHub repository can you include the output when running "npm install" in the project folder?

@EisenbergEffect
Copy link
Author

Here you go:

C:\Users\EisenbergEffect>npm install -g madge
npm http GET https://registry.npmjs.org/madge
npm http 304 https://registry.npmjs.org/madge
npm http GET https://registry.npmjs.org/commander/1.0.0
npm http GET https://registry.npmjs.org/detective/0.1.1
npm http GET https://registry.npmjs.org/walkdir/0.0.5
npm http GET https://registry.npmjs.org/resolve/0.2.3
npm http GET https://registry.npmjs.org/graphviz/0.0.7
npm http GET https://registry.npmjs.org/commondir/0.0.1
npm http GET https://registry.npmjs.org/uglify-js/1.2.6
npm http GET https://registry.npmjs.org/colors/0.6.0-1
npm http GET https://registry.npmjs.org/coffee-script/1.3.3
npm http 304 https://registry.npmjs.org/detective/0.1.1
npm http 304 https://registry.npmjs.org/resolve/0.2.3
npm http 304 https://registry.npmjs.org/walkdir/0.0.5
npm http 304 https://registry.npmjs.org/commander/1.0.0
npm http 304 https://registry.npmjs.org/commondir/0.0.1
npm http 304 https://registry.npmjs.org/uglify-js/1.2.6
npm http 304 https://registry.npmjs.org/graphviz/0.0.7
npm http 304 https://registry.npmjs.org/colors/0.6.0-1
npm http 304 https://registry.npmjs.org/coffee-script/1.3.3
npm http GET https://registry.npmjs.org/temp
npm http 304 https://registry.npmjs.org/temp
C:\Users\EisenbergEffect\AppData\Roaming\npm\madge ->
C:\Users\EisenbergEffect\A
ppData\Roaming\npm\node_modules\madge\bin\madge
[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge
node_modules\commondir

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge\n
ode_modules\colors

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge
node_modules\commander

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge
node_modules\detective

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge\no
de_modules\resolve

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge\no
de_modules\walkdir

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\ma
dge\node_modules\coffee-script

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge
node_modules\uglify-js

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge\node_
modules\graphviz\node_modules\temp

[email protected]:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge\n
ode_modules\graphviz

[email protected] C:\Users\EisenbergEffect\AppData\Roaming\npm\node_modules\madge

(Now I see walkdir...perhaps I just wasn't up to date before?)

On Sun, Feb 3, 2013 at 1:53 PM, Patrik Henningsson <[email protected]

wrote:

Strange .. and you're installing madge with NPM? Can you copy the entire
output when running "npm install madge" and include it here?


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-13051619.

Rob Eisenberg,
Blue Spire Consulting, Inc.
Caliburn Project
850.264.3996

@pahen
Copy link
Owner

pahen commented Feb 3, 2013

Yea, seems correct. Don't know why you had problem before .. but glad it worked for you now :)

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

3 participants