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

--all option fails with ES6 modules #91

Closed
joews opened this issue Dec 11, 2015 · 8 comments
Closed

--all option fails with ES6 modules #91

joews opened this issue Dec 11, 2015 · 8 comments
Labels

Comments

@joews
Copy link

joews commented Dec 11, 2015

--all causes NYC to fail immediately for code with ES6 modules. I am using Node @5.1.0, nyc @5.0.0 with mocha and babel-register.

The error looks like:

/Users/joe/code/nyc-all-issue/node_modules/nyc/node_modules/istanbul/node_modules/esprima/esprima.js:5703
            throw e;
                  ^
Error: Line 1: Unexpected token
    at constructError (/Users/joe/code/nyc-all-issue/node_modules/nyc/node_modules/istanbul/node_modules/esprima/esprima.js:2405:21)
    at createError (/Users/joe/code/nyc-all-issue/node_modules/nyc/node_modules/istanbul/node_modules/esprima/esprima.js:2424:17)
...

.babelrc:

{
  presets: ["es2015"]
}

package.json:

{
  "scripts": {
    "test": "nyc --require babel-register mocha test.js",
    "test-all": "nyc --require babel-register --all mocha test.js",
    "run": "node test.js"
  },
  "dependencies": {
    "babel-core": "^6.3.17",
    "babel-preset-es2015": "^6.3.13",
    "babel-register": "^6.3.13",
    "mocha": "^2.3.4",
    "nyc": "^5.0.0"
  }
}

$ npm test succeeds and $ npm run test-all fails where test.js uses import, or requires a file that uses import.

https://github.com/jwhitfieldseed/nyc-issue-91 reproduces. It runs NYC with/out --all on code with/out import.

It seems other syntax that needs transpiling is OK with or without --all. I tested with rest args, which are not in Node 5.

with import

$ npm run test       // success
$ npm run test-all   // fails unexpectedly
$ npm run run        // SyntaxError, as expected - no rest args or import in Node 5

without import

$ npm run test       // success
$ npm run test-all   // success
$ npm run run        // SyntaxError, as expected - no rest args in Node 5
@novemberborn
Copy link
Contributor

Ohh nice find!

--all bypasses any custom require hooks loaded through --require, so the files are never transpiled before being handed to Istanbul, which then fails to parse the new syntax.

@bcoe is there any insight you could provide as to --all's implementation? It seems to run in the topmost nyc process, not in the mocha subprocess, so simply doing a require(filename) inside addAllFiles() doesn't work either.

@bcoe
Copy link
Member

bcoe commented Dec 12, 2015

@novemberborn @jwhitfieldseed I know Istanbul has its own implementation of this functionality using the configuration setting: --include-all-sources, perhaps we could see about having this functionality exposed when running Istanbul as an API? I'm not quite sure what this would entail CC: @gotwarlost

@bcoe bcoe added the bug label Jan 2, 2016
@albertogasparin
Copy link

I can confirm this bug: --all + --require = 💥
Tried setting include-all-sources: true in .istanbul.yml, but it does nothing.

@bcoe
Copy link
Member

bcoe commented Jan 18, 2016

@albertogasparin mind sending a gist or link to a project with a minimal failing example, I'm on the road for 8 hours today and would love to work on patching this.

@joews
Copy link
Author

joews commented Jan 18, 2016

@bcoe is the repo from the issue useful? npm test-all fails in https://github.com/jwhitfieldseed/nyc-issue-91/tree/master/with-import.

@joews
Copy link
Author

joews commented Jan 18, 2016

@bcoe actually, removing some indirection:

cd with-import
./node_modules/.bin/nyc --require babel-register --all mocha index.js

@bcoe
Copy link
Member

bcoe commented Jan 18, 2016

@jwhitfieldseed 👍 that's a big help.

@bcoe bcoe closed this as completed Jan 24, 2016
@cantidio
Copy link

Hello @bcoe I see that you closed this issue, but this still seems to be happening. Do we have a workaround? thanks

tehsenaus added a commit to tehsenaus/nyc that referenced this issue May 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants