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

Using ES6 code inside require.js module (AMD) causes dependencies to not show #84

Closed
Nuottajarvi opened this issue Jun 12, 2016 · 5 comments

Comments

@Nuottajarvi
Copy link

If you use ES6 code in require.js it shows that the module has no dependencies.

@mrjoelkemp
Copy link
Contributor

Thanks for reporting @Urauth! Mind supplying an example code nippet or a failing test (if you have time)?

@Nuottajarvi
Copy link
Author

Sorry for taking couple days to answer. A for-of loop breaks it for example. I used as an example

var x = [1,2,3,4]

for(var i of x){
x++;
}

@mrjoelkemp
Copy link
Contributor

Thanks for the info @Urauth! Seems like we have an outdated version of amdetective (we're using 0.0.2 while the latest is 0.2.1). There may have been a bump to esprima 2 during that time (which handles es6).

Mind trying to bump the version of amdetective here https://github.com/pahen/madge/blob/master/package.json#L27 locally and seeing if it works? If so, a PR with the update would be very appreciated.

Thanks!

@Nuottajarvi
Copy link
Author

No idea if I did it right or not but I couldn't get it to work. I am pretty much a complete novice on node modules like this.

mrjoelkemp added a commit to mrjoelkemp/madge that referenced this issue Jun 25, 2016
mrjoelkemp added a commit to mrjoelkemp/madge that referenced this issue Jun 25, 2016
mrjoelkemp added a commit to mrjoelkemp/madge that referenced this issue Jun 25, 2016
mrjoelkemp added a commit to mrjoelkemp/madge that referenced this issue Jun 25, 2016
@mrjoelkemp
Copy link
Contributor

@Urauth not a problem! I opened a PR with the fix here: #90.

Some steps that I took:

  1. Check out the commit log for amdetective to make sure that there was nothing concerning between our version and the new version
  2. Run npm install [email protected]
    • Note: if you only update the package.json version number manually, you won't perform the necessary update the npm-shrinkwrap.json file
  3. Run npm test to make sure all tests still pass with the updated package

Then it was a matter of writing a test and submitting the PR.

Once the PR gets merged and a release gets cut, your issue should be fixed. Thanks again for contributing!

@pahen pahen closed this as completed in #90 Jun 28, 2016
pahen pushed a commit that referenced this issue Jun 28, 2016
nmeylan pushed a commit to nmeylan/madge that referenced this issue Jan 7, 2020
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

2 participants