Skip to content

Commit

Permalink
feat(adapter): support npm module names in commitizen.path config
Browse files Browse the repository at this point in the history
This change contains
*   A refactoring of the `adapter.resolveAdapterPath` method

*   An additional test case for the `adapter.resolveAdapterPath` method

*   Updated documentation to reflect the changes

*   Remove unneeded resolving of param passed to getPrompter in git-cz
strategy

*   Use find-root package to determine position of package.json to
determine the path more reliable in complex npm situations

Instead of doing `fs.lsStatSync` calls against `commitizen.path`
`resolveAdapterPath` relies on `require.resolve`, which allows for
support of npm module names in `commitizen.path` while maintaining
backwards compatibility with the former implementation and
documentation.

Closes #79
  • Loading branch information
marionebl authored and jimthedev committed Mar 3, 2016
1 parent d94487f commit 58d492e
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 58 deletions.
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,21 @@ The above command does three things for you. It installs the cz-conventional-cha
...
"config": {
"commitizen": {
"path": "node_modules/cz-conventional-changelog"
"path": "cz-conventional-changelog"
}
}
```

This just tells Commitizen which adapter we actually want our contributors to use when they try to commit to this repo.

`commitizen.path` is resolved via [require.resolve](https://nodejs.org/api/globals.html#globals_require_resolve) and supports

* npm modules
* directories relative to `process.cwd()` containing an `index.js` file
* file base names relative to `process.cwd()` with `js` extension
* full relative file names
* absolute paths.

Please note that in previous version of Commitizen we used czConfig. **czConfig has been deprecated** and you should migrate to the new format before Commitizen 3.0.0.

#### Congratulations your repo is Commitizen-friendly. Time to flaunt it!
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"glob": "7.0.0",
"inquirer": "0.12.0",
"lodash": "4.6.1",
"find-root": "^1.0.0",
"minimist": "1.2.0",
"shelljs": "0.5.3",
"strip-json-comments": "2.0.1"
Expand Down
50 changes: 22 additions & 28 deletions src/cli/strategies/git-cz.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import fs from 'fs';
import path from 'path';
import sh from 'shelljs';
import inquirer from 'inquirer';
import findRoot from 'find-root';
import {getParsedPackageJsonFromPath} from '../../common/util';
import {gitCz as gitCzParser, commitizen as commitizenParser} from '../parsers';
import {commit, staging, adapter} from '../../commitizen';
Expand All @@ -11,8 +12,8 @@ import * as gitStrategy from './git';
// destructure for shorter apis
let { parse } = gitCzParser;

let { getNearestNodeModulesDirectory, getNearestProjectRootDirectory, getPrompter } = adapter;
let { isClean } = staging;
let { getPrompter, resolveAdapterPath } = adapter;
let { isClean } = staging;

export default gitCz;

Expand All @@ -37,31 +38,24 @@ function gitCz(rawGitArgs, environment, adapterConfig) {

// Now, if we've made it past overrides, proceed with the git-cz strategy
let parsedGitCzArgs = parse(rawGitArgs);

// TODO: This can be broken out into its own function.
// Basically we're
// 1. Walking up the tree to find a node_modules folder
// 2. Resolving the project root based on the node_modules folder
// 3. Resolving the adapter bath based on that project root
let resolvedAdapterConfigPath = path.join(getNearestProjectRootDirectory(), adapterConfig.path);

let prompter = getPrompter(path.resolve(process.cwd(), resolvedAdapterConfigPath));
let resolvedAdapterConfigPath = resolveAdapterPath(adapterConfig.path);
let resolvedAdapterRootPath = findRoot(resolvedAdapterConfigPath);
let prompter = getPrompter(adapterConfig.path);

isClean(process.cwd(), function(stagingIsClean){
if(stagingIsClean) {
console.error('Error: No files added to staging! Did you forget to run git add?')
} else {

// OH GOD IM SORRY FOR THIS SECTION
let adapterPackageJson = getParsedPackageJsonFromPath(resolvedAdapterConfigPath);
let cliPackageJson = getParsedPackageJsonFromPath(environment.cliPath);
console.log(`cz-cli@${cliPackageJson.version}, ${adapterPackageJson.name}@${adapterPackageJson.version}\n`);
commit(sh, inquirer, process.cwd(), prompter, {args: parsedGitCzArgs, disableAppendPaths:true, emitData:true, quiet:false}, function() {
// console.log('commit happened');
});

}
});

isClean(process.cwd(), function(stagingIsClean){
if(stagingIsClean) {
console.error('Error: No files added to staging! Did you forget to run git add?')
} else {
// OH GOD IM SORRY FOR THIS SECTION
let adapterPackageJson = getParsedPackageJsonFromPath(resolvedAdapterRootPath);
let cliPackageJson = getParsedPackageJsonFromPath(environment.cliPath);
console.log(`cz-cli@${cliPackageJson.version}, ${adapterPackageJson.name}@${adapterPackageJson.version}\n`);
commit(sh, inquirer, process.cwd(), prompter, {args: parsedGitCzArgs, disableAppendPaths:true, emitData:true, quiet:false}, function() {
// console.log('commit happened');
});

}
});


}
}
46 changes: 20 additions & 26 deletions src/commitizen/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ export {

/**
* ADAPTER
*
* Adapter is generally responsible for actually installing adapters to an
*
* Adapter is generally responsible for actually installing adapters to an
* end user's project. It does not perform checks to determine if there is
* a previous commitizen adapter installed or if the proper fields were
* provided. It defers that responsibility to init.
* provided. It defers that responsibility to init.
*/

/**
Expand Down Expand Up @@ -107,44 +107,38 @@ function getNpmInstallStringMappings(save, saveDev, saveExact, force) {
* Gets the prompter from an adapter given an adapter path
*/
function getPrompter(adapterPath) {
// We need to handle directories and files, so resolve the parh first
// Resolve the adapter path
let resolvedAdapterPath = resolveAdapterPath(adapterPath);

// Load the adapter
let adapter = require(resolvedAdapterPath);

if(adapter && adapter.prompter && isFunction(adapter.prompter)) {
return adapter.prompter;
return adapter.prompter;
} else {
throw "Could not find prompter method in the provided adapter module: " + adapterPath;
}
}

/**
* Given a path, which can be a directory or file, will
* Given a resolvable module name or path, which can be a directory or file, will
* return a located adapter path or will throw.
*/
function resolveAdapterPath(inboundAdapterPath) {
let outboundAdapterPath;
// Check if inboundAdapterPath is a path or node module name
let parsed = path.parse(inboundAdapterPath);
let isPath = parsed.dir.length > 0;

// Resolve from process.cwd() if inboundAdapterPath is a path
let absoluteAdapterPath = isPath ?
path.resolve(process.cwd(), inboundAdapterPath) :
inboundAdapterPath;

// Try to open the provided path
try {

// If we're given a directory, append index.js
if(fs.lstatSync(inboundAdapterPath).isDirectory()) {

// Modify the path and make sure the modified path exists
outboundAdapterPath = path.join(inboundAdapterPath, 'index.js');
fs.lstatSync(outboundAdapterPath);

} else {
// The file exists and is a file, so just return it
outboundAdapterPath = inboundAdapterPath;
}
return outboundAdapterPath;

} catch(err) {
throw err;
// try to resolve the given path
return require.resolve(absoluteAdapterPath);
} catch (error) {
error.message = "Could not resolve " + absoluteAdapterPath, ". " + error.message;
throw error;
}

}
}
5 changes: 2 additions & 3 deletions test/tests/adapter.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {expect} from 'chai';
import path from 'path';
import fs from 'fs';

// TODO: augment these tests with tests using the actual cli call
// For now we're just using the library, which is probably fine
Expand Down Expand Up @@ -60,8 +59,8 @@ describe('adapter', function() {
// TEST
expect(function() {adapter.resolveAdapterPath('IAMANIMPOSSIBLEPATH'); }).to.throw(Error);
expect(function() {adapter.resolveAdapterPath(adapterConfig.path); }).not.to.throw(Error);
expect(function() {adapter.resolveAdapterPath(path.join(adapterConfig.path, 'index.js')) }).not.to.throw(Error);

expect(function() {adapter.resolveAdapterPath(path.join(adapterConfig.path, 'index.js')); }).not.to.throw(Error);
expect(function() {adapter.resolveAdapterPath('cz-conventional-changelog'); }).not.to.throw(Error);
});

it('gets adapter prompter functions', function(){
Expand Down

0 comments on commit 58d492e

Please sign in to comment.