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

ES6 tests failing #61

Closed
tafsiri opened this issue Jun 18, 2015 · 14 comments
Closed

ES6 tests failing #61

tafsiri opened this issue Jun 18, 2015 · 14 comments

Comments

@tafsiri
Copy link

tafsiri commented Jun 18, 2015

I was trying to analyse dependencies written in es6 syntax and had some trouble and then discovered that the es6 related tests are failing. This is on master sha 3b4957b

after checkout out the repo I ran npm install and npm test and got the following output.

  module format (ES6)
    1) should behave as expected on ok files
    ✓ should tackle errors in files 
    2) should be able to exclude modules
    3) should find circular dependencies
    4) should find absolute imports from the root

...

  1) module format (ES6) should behave as expected on ok files:

      AssertionError: expected Object {
  a: Array [],
  d: Array [],
  'fancy-main/not-index': Array [],
  'sub/b': Array [],
  'sub/c': Array []
} to equal Object {
  a: Array [ 'sub/b' ],
  d: Array [],
  'fancy-main/not-index': Array [],
  'sub/b': Array [ 'sub/c' ],
  'sub/c': Array [ 'd' ]
} (at a -> length, A has 0 and B has 1)
      + expected - actual

       {
      +  "a": [
      +    "sub/b"
      +  ],
      -  "a": [],
         "d": [],
         "fancy-main/not-index": [],
      +  "sub/b": [
      +    "sub/c"
      +  ],
      +  "sub/c": [
      +    "d"
      +  ]
      -  "sub/b": [],
      -  "sub/c": []
       }

(there is more output, but i figure this should be easy to reproduce).
Thanks

@rafayepes
Copy link

Why not use Babel to parse the files? Getting some issues with ES6 as well.

@pahen
Copy link
Owner

pahen commented Oct 16, 2015

What version of Node.js are you running and on what platform?

@pahen pahen added question and removed question labels Oct 16, 2015
@rafayepes
Copy link

I'm using grunt-madge v0.0.6 in Node v0.12.2.

I'm getting parsing errors with things like arrow functions, for..Of, template strings,...

"Error while parsing file: foo.js"
"Warning: Line 13: Unexpected token =>"

I have no issues using let and const.

EDIT: I can run all tests in green. If I change the es6 examples to use arrow functions, they still work. However, if I change the AMD examples to use arrow functions they break.

1) module format (AMD) should resolve relative module indentifiers:

      AssertionError: expected Object {
  a: Array [],
  b: Array [],
  'foo/bar/d': Array [ 'a' ],
  'foo/c': Array [ 'a' ]
} to equal Object {
  a: Array [],
  b: Array [ 'a' ],
  'foo/bar/d': Array [ 'a' ],
  'foo/c': Array [ 'a' ]
} (at b -> length, A has 0 and B has 1)
      + expected - actual

       {
         "a": []
      -  "b": []
      +  "b": [
      +    "a"
      +  ]
         "foo/bar/d": [
           "a"
         ]
         "foo/c": [

      at Assertion.fail (node_modules/should/lib/assertion.js:180:17)
      at Assertion.prop.value (node_modules/should/lib/assertion.js:65:17)
      at Context.<anonymous> (test/amd.js:105:19)

Might it be that when using AMD, there is a different parser being use or something like that? My project uses AMD for module loading but uses other ES6 features. I believe amdetective doesn't support ES6 features.

@rafayepes
Copy link

I believe my issues with AMD and ES6 this should be fixed by mixu/amdetective#3

@rafayepes
Copy link

Can we update amdetective dependency to v0.1.0? Would be nice to update grunt-madge as well to include ES6 support for AMD modules as well.

Thanks!

@mrjoelkemp
Copy link
Contributor

I have an alternate detective for amd that also has es6 and jsx support (uses acorn under the hood): https://github.com/mrjoelkemp/node-detective-amd. In addition, it accepts as AST (or a file's contents) to avoid double parsing. Happy to help integrate it into madge.

@mrjoelkemp
Copy link
Contributor

Btw, madge is also using my detective-es6 plugin which also supports jsx and an AST source. Just as an fyi, I wrote precinct: https://github.com/mrjoelkemp/node-precinct to avoid worrying about module types and get the dependencies of any JS file (es6, cjs, amd) with an AST or file content source. It also supports SASS/Stylus (Less would be easy to add). It's a detective factory. May remove a lot of logic from madge core.

@deanrad
Copy link

deanrad commented May 3, 2016

@mrjoelkemp - Precinct === Detective Factory - that's hilarious. I'm stuck on JSX right now—I'll tie my work to this issue, and see if using precinct (at least for JSX) helps..

@mrjoelkemp
Copy link
Contributor

@deanius JSX with ES6? Try updating the detective-es6 version in madge to 1.1.3 since 1.1.2 didn't have JSX support. Let me know if that works.

deanrad pushed a commit to deanrad/madge that referenced this issue May 3, 2016
deanrad pushed a commit to deanrad/madge that referenced this issue May 3, 2016
deanrad pushed a commit to deanrad/madge that referenced this issue May 3, 2016
deanrad pushed a commit to deanrad/madge that referenced this issue May 3, 2016
deanrad pushed a commit to deanrad/madge that referenced this issue May 5, 2016
Issue pahen#61

prefer detective-es6-1.1.3

Revert to esold

Updated shrinkwrap
@deanrad
Copy link

deanrad commented May 24, 2016

@mrjoelkemp - any thoughts on how to do this with files that need babel precompilation? I've got react files that use object-rest-spread babel plugin and they break madge.

@rafayepes
Copy link

@deanius you should probably compile those files with Babel first. Then, point madge to the folder were your Babel output files are. That should do the trick.

@mrjoelkemp
Copy link
Contributor

Detective-es6 could benefit from using babylon (babel's parser) instead of Acorn directly for parsing files that use features at various acceptance stages.

This would be a breaking (but I'm definitely open to it) change in node-source-walk. After that, we'd push the version bump to detective-es6 and then bump the version of detective-es6 in madge.

Another option is to have node-source-walk accept a configurable parser (with acorn as the default). This seems unnecessary since babylon is a superset of Acorn (if I remember correctly).

For now, precompiling your code is a work around.

Ref dependents/node-source-walk#14

@deanrad
Copy link

deanrad commented May 26, 2016

Thanks @rafayepes @mrjoelkemp

@mrjoelkemp
Copy link
Contributor

I made the fix to detective-es6 (more specifically to node-source-walk) to support all available ES7 plugins: https://github.com/babel/babylon#plugins. The goal is to always be able to parse a JS file for the dependencies – regardless of the syntax.

This should be fixed when #83 is merged.

@pahen pahen closed this as completed in #83 Jun 13, 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

5 participants