-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: remove
karma
binary in favor of karma-cli
BREAKING CHANGE: The `karma` module does not export `karma` binary anymore. The recommended way is to have local modules (karma and all the plugins that your project needs) stored in your `package.json`. You can run that particular Karma by `./node_modules/karma/bin/karma`. Or you can have `karma-cli` installed globally on your system, which enables you to use the `karma` command. The global `karma` command (installed by `karma-cli`) does look for local version of Karma (including parent directories) first and fall backs to a global one. The `bin/karma` binary does not look for any other instances of Karma and just runs the one that it belongs to.
- Loading branch information
Showing
3 changed files
with
23 additions
and
31 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,3 @@ | ||
#!/usr/bin/env node | ||
|
||
var path = require('path'); | ||
var fs = require('fs'); | ||
|
||
// Try to find a local install | ||
var dir = path.resolve(process.cwd(), 'node_modules', 'karma', 'lib'); | ||
|
||
// Check if the local install exists else we use the install we are in | ||
if (!fs.existsSync(dir)) { | ||
dir = path.join('..', 'lib'); | ||
} | ||
|
||
var cli = require(path.join(dir, 'cli')); | ||
var config = cli.process(); | ||
|
||
switch (config.cmd) { | ||
case 'start': | ||
require(path.join(dir, 'server')).start(config); | ||
break; | ||
case 'run': | ||
require(path.join(dir, 'runner')).run(config); | ||
break; | ||
case 'init': | ||
require(path.join(dir, 'init')).init(config); | ||
break; | ||
case 'completion': | ||
require(path.join(dir, 'completion')).completion(config); | ||
break; | ||
} | ||
require('../lib/cli').run(); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
c7d4627
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vojtajina this seems to be causing some issues running tests on windows, https://github.com/angular/templating and https://github.com/caitp/watchtower.js both suffer from this. Exporting the binaries creates a
.cmd
wrapper file on windows, which is necessary as windows ignores the shebang line in binaries.I'm not sure if this property of the configuration is deprecated or not (in which case this should be an NPM issue), but this seems to be causing problems, and it would be great to find out if there's a proper way around this.
c7d4627
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this has broken npm-scripts aliasing, so you can't do this in a package anymore:
package.json:
Normally that would run the local binary.
c7d4627
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for this change?
c7d4627
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@necolas you can install karma-cli locally to the project
and use
karma start
in package.json@sudodoki couple of reasons:
c7d4627
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify: I'm mostly interested in why would binary not be put into
node_modules/.bin/karma
, but be kept innode_modules/karma/bin/karma
.c7d4627
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm installs files specified as
bin
in package.json files into anode_modules/.bin
directory which is a stand-in for$NODE_PREFIX/bin
e.g./usr/local/bin
,/usr/bin
, etc.. When packages that specify anything asbin
are installed outside of the global directory, this is what happens.c7d4627
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A user's informed me
./node_modules/karma/bin/karma
doesn't work in powershell and we aren't going to force users to install karma globally...c7d4627
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@megawac you can install
krma-cli
locale to the project and usenpm test