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

Conflict between ava and nyc when nyc uses @babel/register. #2089

Closed
coreyfarrell opened this issue Apr 7, 2019 · 5 comments
Closed

Conflict between ava and nyc when nyc uses @babel/register. #2089

coreyfarrell opened this issue Apr 7, 2019 · 5 comments

Comments

@coreyfarrell
Copy link
Contributor

Description

When something causes @babel/register to be required to the ava process this can cause a parse error for ava.config.js.

Test Source

N/A

Error Message & Stack Trace

✖ Error loading ava.config.js

/usr/src/npm/failures/nyc-ava/node_modules/ava/lib/load-config.js:17
ReferenceError: default is not defined
    at loadConfig (/usr/src/npm/failures/nyc-ava/node_modules/ava/lib/load-config.js:17:5)
    at Object.exports.run (/usr/src/npm/failures/nyc-ava/node_modules/ava/lib/cli.js:24:10)
    at Object.<anonymous> (/usr/src/npm/failures/nyc-ava/node_modules/ava/cli.js:10:23)
    at Module._compile (internal/modules/cjs/loader.js:701:30)
    at Module.replacementCompile (/usr/src/npm/failures/nyc-ava/node_modules/nyc/node_modules/append-transform/index.js:58:13)
    at Module._compile (/usr/src/npm/failures/nyc-ava/node_modules/pirates/lib/index.js:99:24)
    at Module._extensions..js (internal/modules/cjs/loader.js:712:10)
    at /usr/src/npm/failures/nyc-ava/node_modules/nyc/node_modules/append-transform/index.js:62:4
    at newLoader (/usr/src/npm/failures/nyc-ava/node_modules/pirates/lib/index.js:104:7)
    at Object.<anonymous> (/usr/src/npm/failures/nyc-ava/node_modules/nyc/node_modules/append-transform/index.js:62:4)

Config

Any ava.config.js ES module, even an empty config:

export default {};

babel.config.js with @babel/plugin-transform-modules-commonjs:

'use strict';

module.exports = {
	plugins: [
		'@babel/plugin-transform-modules-commonjs'
	]
};

Command-Line Arguments

Two commands can reproduce this:

nyc --require @babel/register ava
NODE_OPTIONS='--require=@babel/register' ava

Note the NODE_OPTIONS method is simply a way to show the problem isolated from nyc.

It is sometimes mandatory for --require @babel/register to be given to nyc. WIthout this it is impossible for nyc to generate coverage data for sources that were never loaded (nyc --all).

Work around

Adding ignore: ["ava.config.js"] to babel.config.js seems to make this issue go away.

Relevant Links

See https://github.com/coreyfarrell/nyc-ava for minimal demo of issue. npm test and npm run no-nyc can be used to initiate the failure.

Environment

$ node -e "var os=require('os');console.log('Node.js ' + process.version + '\n' + os.platform() + ' ' + os.release())"
Node.js v10.15.3
linux 4.20.15-100.fc28.x86_64
$ npx ava --version
1.4.1
$ npm --version
6.4.1
@novemberborn
Copy link
Member

Adding ignore: ["ava.config.js"] to babel.config.js seems to make this issue go away.

That's my preferred solution. Happy to take a PR to the documentation / recipes which brings up this edge-case.

Ultimately you're changing the environment AVA is running in, breaking its assumptions. I don't think we need to cater to that.

(I'm closing this issue for housekeeping purposes, but let's keep the conversation going.)

@coreyfarrell
Copy link
Contributor Author

CC @kevinji.

@novemberborn this seems reasonable to me. Maybe it's possible/useful to capture the error and produce a more specific error from ava instead of letting the user see the node.js module loader stack trace?

@novemberborn
Copy link
Member

Maybe it's possible/useful to capture the error and produce a more specific error from ava instead of letting the user see the node.js module loader stack trace?

Yes, though looking at the code I'm not even sure how that error would occur:

({default: fileConf = MISSING_DEFAULT_EXPORT} = esm(module, {

@coreyfarrell
Copy link
Contributor Author

@kevinji @novemberborn I've investigated further, what we are seeing is standard-things/esm#773. So it looks like this will be fixed in the next release of esm. I've verified that with esm@3.2.20 it's OK if @babel/register translates ava.config.js to CJS. I also verified the issue is resolved with a locally built copy of esm#master.

As an alternative @kevinji you could pin esm in your own package, "esm": "=3.2.20 || ^3.2.23" in your dependencies will prevent ava from pulling in the broken version, yet allow it to eventually upgrade. This way you can leave your babel config alone (it won't be required to ignore ava.config.js).

@kevinji
Copy link

kevinji commented Apr 12, 2019

@coreyfarrell Sweet, thanks for spending the time to look into the issue!

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