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.

Sign up for GitHub

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

Jump to bottom

On Node 10 and older, CLI can't find "qunit" if there's a "qunit.json" file #1484

Closed
Krinkle opened this issue Sep 30, 2020 · 2 comments · Fixed by #1485
Closed

On Node 10 and older, CLI can't find "qunit" if there's a "qunit.json" file #1484

Krinkle opened this issue Sep 30, 2020 · 2 comments · Fixed by #1485
Assignees
@Krinkle
Labels
Component: CLI Type: Bug Something isn't working right.

Comments

@Krinkle
Copy link
Member

Krinkle commented Sep 30, 2020

Tell us about your runtime:

  • QUnit version: 2.11.2
  • What environment are you running QUnit in? (e.g., browser, Node): Node

What are you trying to do?

Running qunit test/foo.js via npm test from package.json.

The project directory is a clone of https://github.com/wikimedia/eslint-config-wikimedia/.

What actually happened?

The QUnit CLI process fails early on due to confusing this file for the qunit package:

/eslint-config-wikimedia/node_modules/js-reporters/dist/js-reporters.js:1530
    runner.on('runStart', this.onRunStart.bind(this));
           ^

TypeError: runner.on is not a function
    at new TapReporter (/eslint-config-wikimedia/node_modules/js-reporters/dist/js-reporters.js:1530:12)
    at Function.init (/eslint-config-wikimedia/node_modules/js-reporters/dist/js-reporters.js:1598:14)
    at run (/eslint-config-wikimedia/node_modules/qunit/src/cli/run.js:45:19)
    at Object.<anonymous> (/eslint-config-wikimedia/node_modules/qunit/bin/qunit.js:56:2)

Manual inspection reveals that the runner variable received from https://github.com/qunitjs/qunit/blob/2.11.2/src/cli/require-qunit.js#L7 is in fact the qunit.json file, and not the qunit package.

		const localQUnitPath = resolve( "qunit", { paths: [ process.cwd() ] } );
@Krinkle Krinkle added the Type: Bug Something isn't working right. label Sep 30, 2020
@Krinkle Krinkle self-assigned this Sep 30, 2020
@Krinkle
Copy link
Member Author

Krinkle commented Sep 30, 2020

I raised this upstream with Node because it seems to me like a bug.

  • Calling require('qunit') directly from that same working directory does not return ./qunit.json. It only happens when using the paths option.
  • It only fails on Node 10. It works fine on Node 14.

See nodejs/node#35367

@Krinkle
Copy link
Member Author

Krinkle commented Sep 30, 2020

Suggested workaround for now, until we drop Node 10 support:

> process.versions.node
10.15.2

> require.resolve( "qunit", { paths: [ process.cwd() ] } )
'/eslint-config-wikimedia/qunit.json'

> require.resolve( "qunit", { paths: [ process.cwd() + '/node_modules', process.cwd() ] } )
'/eslint-config-wikimedia/node_modules/qunit/qunit/qunit.js'

Krinkle added a commit that referenced this issue Sep 30, 2020
@Krinkle
CLI: Fix 'qunit' require error on Node 10 if qunit.json exists
6860882 
If the project using QUnit has a local qunit.json file in the
repository root, then the CLI was unable to find the 'qunit'
package. Instead, it first found a local 'qunit.json' file.

This affected an ESLint plugin with a 'qunit' preset file.

This bug was fixed in Node 12, and thus only affects Node 10 for us.

The bug is also specific to require.resolve(). Regular use of
require() was not affected and always correctly found the
local package. (A local file would only be considered if the
require started with dot-slash like `./qunit`.)

Ref nodejs/node#35367.
Fixes #1484.
@Krinkle Krinkle mentioned this issue Sep 30, 2020
Merged
Krinkle added a commit that referenced this issue Oct 3, 2020
@Krinkle
CLI: Fix 'qunit' require error on Node 10 if qunit.json exists
f0117c0 
If the project using QUnit has a local qunit.json file in the
repository root, then the CLI was unable to find the 'qunit'
package. Instead, it first found a local 'qunit.json' file.

This affected an ESLint plugin with a 'qunit' preset file.

This bug was fixed in Node 12, and thus only affects Node 10 for us.

The bug is also specific to require.resolve(). Regular use of
require() was not affected and always correctly found the
local package. (A local file would only be considered if the
require started with dot-slash like `./qunit`.)

Ref nodejs/node#35367.
Fixes #1484.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Krinkle Krinkle

Labels
Component: CLI Type: Bug Something isn't working right.
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

CLI: Fix 'qunit' require error on Node 10 if qunit.json exists qunitjs/qunit
1 participant
@Krinkle