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

qunit --require does not find local module if qunit is globally installed #1372

Closed
ghost opened this issue Jan 24, 2019 · 10 comments
Closed
Assignees
Labels
Component: CLI Type: Bug Something isn't working right.

Comments

@ghost
Copy link

ghost commented Jan 24, 2019

Tell us about your runtime:

  • QUnit version: 2.9.1
  • What environment are you running QUnit in? (e.g., browser, Node): Node v11.7.0
  • How are you running QUnit? (e.g., script, testem, Grunt): CLI

What are you trying to do?

Pull request #1271 added the --require option, but it does not seem to work with esm, as @jdalton hoped.

I wrote a test of using esm with qunit --require. Perhaps I am not doing it correctly. If you can help me figure this out, I would happily contribute to qunitjs/qunitjs.com#144.

npm install to get esm package.

npm install -g qunit to get qunit globally.

qunit --require esm test.js to run the test.

What actually happened?

TAP version 13
not ok 1 test.js > Failed to load the test file with error:
/Users/jeremiah/Projects/esm-qunit-experiment/test.js:1
(function (exports, require, module, __filename, __dirname) { import {bar} from './foo.js';
                                                                     ^

SyntaxError: Unexpected token {
    at new Script (vm.js:84:7)
    at createScript (vm.js:264:10)
    at Object.runInThisContext (vm.js:312:10)
    at Module._compile (internal/modules/cjs/loader.js:684:28)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:732:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
    at Function.Module._load (internal/modules/cjs/loader.js:552:3)
    at Module.require (internal/modules/cjs/loader.js:657:17)
    at require (internal/modules/cjs/helpers.js:22:18)
  ---
  message: "should be able to load file"
  severity: failed
  actual: false
  expected: true
  stack:     at Object.<anonymous> (/usr/local/lib/node_modules/qunit/src/cli/run.js:60:13)
  ...
1..1
# pass 0
# skip 0
# todo 0
# fail 1

What did you expect to happen?

TAP version 13
ok 1 Hinkle finkle dinkle doo
1..1
# pass 1
# skip 0
# todo 0
# fail 0
@platinumazure
Copy link
Contributor

Hi @jeremiahlee, thanks for the issue.

Just out of curiosity, does it work if both QUnit and esm are installed locally or globally? Asking because ESLint has a similar issue where a global ESLint can't find local plugins.

@ghost
Copy link
Author

ghost commented Jan 24, 2019

Good question, @platinumazure. I hadn't tried those variations.

I tried all globally first:

npm uninstall esm
npm install -g esm
qunit --require esm test.js

I got the same error.

Then I tried installing esm and qunit locally:

npm uninstall -g esm
npm uninstall -g qunit
npm install esm
npm install qunit
npx qunit --require esm test.js

and it worked!

TAP version 13
ok 1 Hinkle finkle dinkle doo
1..1
# pass 1
# skip 0
# todo 0
# fail 0

So good news! I'm unblocked. But is this expected behavior?

@jdalton
Copy link

jdalton commented Jan 24, 2019

Hi @jeremiahlee 👋

Thanks for trying esm with QUnit!
I can speak a little on the resolution front. I believe --require works for local modules, so your npm i esm use is ok, because Node itself cannot find globally installed packages using require().

@ghost
Copy link
Author

ghost commented Jan 24, 2019

@jdalton Thanks for jumping in! I first tried esm locally with global qunit. Are you aware of anything with Node that would prevent global qunit resolving local esm?

@jdalton
Copy link

jdalton commented Jan 24, 2019

Are you aware of anything with Node that would prevent global qunit resolving local esm?

Without QUnit doing some coding require() use in a globally installed package would not resolve local packages. So this may be something QUnit may extend support for using require.resolve with the options param (added in Node 8.9.0) to resolve packages from the process.cwd().

@martypdx
Copy link

I'm not able to get it work with a pure local install:

~/classes/vanilla-web/coin-toss > npm ls -g esm
/Users/marty/.node/lib
└── (empty)

~/classes/vanilla-web/coin-toss > npm ls -g qunit
/Users/marty/.node/lib
└── (empty)

~/classes/vanilla-web/coin-toss > npm ls esm qunit
/Users/marty/classes/vanilla-web/coin-toss
├── [email protected] 
└── [email protected] 

~/classes/vanilla-web/coin-toss > npx qunit --require esm test/tests.js
TAP version 13
not ok 1 test/tests.js > Failed to load the test file with error:
/Users/marty/classes/vanilla-web/coin-toss/test/tests.js:2
import getHeadsOrTails from '../src/get-heads-or-tails.js';
       ^^^^^^^^^^^^^^^

SyntaxError: Unexpected identifier
    at new Script (vm.js:74:7)
    at createScript (vm.js:246:10)
    at Object.runInThisContext (vm.js:298:10)
    at Module._compile (internal/modules/cjs/loader.js:670:28)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:713:10)
    at Module.load (internal/modules/cjs/loader.js:612:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:551:12)
    at Function.Module._load (internal/modules/cjs/loader.js:543:3)
    at Module.require (internal/modules/cjs/loader.js:650:17)
    at require (internal/modules/cjs/helpers.js:20:18)
  ---
  message: "should be able to load file"
  severity: failed
  actual: false
  expected: true
  stack:     at Object.<anonymous> (/Users/marty/classes/vanilla-web/coin-toss/node_modules/qunit/src/cli/run.js:60:13)
  ...
1..1
# pass 0
# skip 0
# todo 0
# fail 1

@martypdx
Copy link

Work around is to use an intermediate file index.js file and use the "manual" esm method:

/* eslint-disable */
require = require('esm')(module);
module.exports = require("./tests.js");

Then launch that file:

> npx qunit tests/index.js

@Krinkle Krinkle added Type: Bug Something isn't working right. Component: CLI labels Feb 23, 2019
@Krinkle Krinkle changed the title qunit --require esm does not work qunit --require does not find local module if qunit is globally installed Apr 13, 2019
@Krinkle
Copy link
Member

Krinkle commented Apr 13, 2019

I think a locally installed qunit should not find a globally installed module. Aside from not having a clear use case, it also seems unexpected. Might obscure issues locally you'd then find elsewhere.

However, the case of globally installing qunit and finding a local module seems one I expect to work.

I do generally recommend for project management to keep versioning of dependencies local to a project (and run via npx or npm run). Nonetheless, installing qunit globally is something we support and I'd expect that to work with local modules.

I've updated the task summary to reflect that use case to distinguish it from the case where both are local, which appears to work.

@Krinkle
Copy link
Member

Krinkle commented Jul 21, 2020

Are you aware of anything with Node that would prevent global qunit resolving local esm?

Without QUnit doing some coding require() use in a globally installed package would not resolve local packages.

So this may be something QUnit may extend support for using require.resolve with the options param (added in Node 8.9.0) to resolve packages from the process.cwd().

This makes sense to me. I'm looking into this now that we're dropping Node 8 support. I'm seeing however that we already do exactly as you suggest. The qunit --require feature as landed in QUnit 2.6 (c6365ed, #1271) already does this. It currently uses node-resolve to support Node 6, but I've changed this to native require.resolve (pull #1464). But either way it already resolves the required packages from the local current working directory and there is an integration test for this as well.

c6365ed6#diff-3309eba0612e0921bb3a101c1bc467ea

function requireFromCWD( mod ) {
	const resolvedPath = resolve.sync( mod, { basedir: process.cwd() } );
	return require( resolvedPath );
};

# qunit/src/cli.js
 	options.requires.forEach( requireFromCWD ); 

Which means that from what I can tell both of the following should work as expected:

  1. A project with qunit and the required package as local dependencies (best practice). But comment qunit --require does not find local module if qunit is globally installed #1372 (comment) suggests this doesn't work.
  2. A developer having qunit installed globally and invoking the local tests directly requiring a local package. Which the original comment suggest doesn't work.

@Krinkle
Copy link
Member

Krinkle commented Aug 31, 2020

Closing for now as this seems to work on 2.11 from my own testing. Feel free to ping me here or file a new issue if you still encounter an issue with --require finding a local modulle when invoking a globally installed qunit.

@Krinkle Krinkle closed this as completed Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: CLI Type: Bug Something isn't working right.
Development

No branches or pull requests

4 participants