Skip to content

Add support for configuration files with ".jsonc" extension #3760

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

Merged
boneskull merged 5 commits into from
Feb 27, 2019
Merged

Add support for configuration files with ".jsonc" extension #3760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48ba0ed
config: default support of .jsonc extension (#3753)
Feb 21, 2019
cfa94e7
Documentation update for #3753
Feb 21, 2019
e847424
Drop JSON5 support
Feb 22, 2019
cdd3538
Fixes JSONC example (no more JSON5)
sstephant Feb 24, 2019
fdf7375
Adds emphasize on the config file extensions
sstephant Feb 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1627,7 +1627,7 @@ In addition to supporting the legacy [`mocha.opts`](#mochaopts) run-control form

- **JavaScript**: Create a `.mocharc.js` in your project's root directory, and export an object (`module.exports = {/* ... */}`) containing your configuration.
- **YAML**: Create a `.mocharc.yaml` (or `.mocharc.yml`) in your project's root directory.
- **JSON**: Create a `.mocharc.json` in your project's root directory. Comments — while not valid JSON — are allowed in this file, and will be ignored by Mocha.
- **JSON**: Create a `.mocharc.json` (or `.mocharc.jsonc`) in your project's root directory. Comments — while not valid JSON — are allowed in this file, and will be ignored by Mocha.
- **`package.json`**: Create a `mocha` property in your project's `package.json`.

Mocha suggests using one of the above strategies for configuration instead of the legacy `mocha.opts` format.
Expand All @@ -1649,6 +1649,7 @@ If no custom path was given, and if there are multiple configuration files in th
1. `.mocharc.js`
1. `.mocharc.yaml`
1. `.mocharc.yml`
1. `.mocharc.jsonc`
1. `.mocharc.json`

### Merging
Expand Down
14 changes: 14 additions & 0 deletions example/config/.mocharc.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// This config file contains Mocha's defaults.
// As you can see, comments are allowed.
// This same configuration could be provided in the `mocha` property of your
// project's `package.json`.
{
"diff": true,
"extension": ["js"],
"opts": "./test/mocha.opts",
"package": /* 📦 */ "./package.json",
"reporter": /* 📋 */ "spec",
"slow": 75,
"timeout": 2000,
"ui": "bdd"
}
1 change: 1 addition & 0 deletions lib/cli/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ exports.CONFIG_FILES = [
'.mocharc.js',
'.mocharc.yaml',
'.mocharc.yml',
'.mocharc.jsonc',
'.mocharc.json'
];

Copy link
Contributor

@plroebuck plroebuck Feb 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat related -- asking for opinions here... not changes atm.

Think this is better?

exports.loadConfig = filepath => {
  let config = {};
  const ext = path.extname(filepath);
  try {
    if (/\.ya?ml/.test(ext)) {
      config = parsers.yaml(filepath);
    } else if (ext === '.js') {
      config = parsers.js(filepath);
    } else if (/\.jsonc?/.test(ext)) {
      config = parsers.json(filepath);
    } else {
      throw new Error('no parser for this type file');
    }
  } catch (err) {
    throw new Error(`failed to parse "${filepath}": ${err}`);
  }
  return config;
};

Rationale: However stupid, if I used --config with a Word document argument, the code would throw a SyntaxError (as result of failing JSON.parse) which is utterly unhelpful and its error message doesn't address the actual problem.

Copy link
Contributor Author

@sstephant sstephant Feb 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the fallback to the JSON parser can lead to an unhelpful error message.
But in your snippet the inner throw would be hidden by the upper catch was that your intention?
Another point I noticed is that the file extension is case sensitive. Was that on purpose?

Copy link
Contributor

@plroebuck plroebuck Feb 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Threw from try just to pass message to catch where it can generate higher-level Error.

Copy link
Contributor

@plroebuck plroebuck Feb 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another point I noticed is that the file extension is case sensitive. Was that on purpose?

We support lowercase file extensions.

Expand Down
19 changes: 15 additions & 4 deletions test/node-unit/cli/config.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('cli/config', function() {
sandbox.stub(parsers, 'js').returns(config);
});

describe('when supplied a filepath with .yaml extension', function() {
describe('when supplied a filepath with ".yaml" extension', function() {
const filepath = 'foo.yaml';

it('should use the YAML parser', function() {
Expand All @@ -35,7 +35,7 @@ describe('cli/config', function() {
});
});

describe('when supplied a filepath with .yml extension', function() {
describe('when supplied a filepath with ".yml" extension', function() {
const filepath = 'foo.yml';

it('should use the YAML parser', function() {
Expand All @@ -46,7 +46,7 @@ describe('cli/config', function() {
});
});

describe('when supplied a filepath with .js extension', function() {
describe('when supplied a filepath with ".js" extension', function() {
const filepath = 'foo.js';

it('should use the JS parser', function() {
Expand All @@ -57,7 +57,18 @@ describe('cli/config', function() {
});
});

describe('when supplied a filepath with .json extension', function() {
describe('when supplied a filepath with ".jsonc" extension', function() {
const filepath = 'foo.jsonc';

it('should use the JSON parser', function() {
loadConfig('foo.jsonc');
expect(parsers.json, 'to have calls satisfying', [
{args: [filepath], returned: config}
]).and('was called times', 1);
});
});

describe('when supplied a filepath with ".json" extension', function() {
const filepath = 'foo.json';

it('should use the JSON parser', function() {
Expand Down