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

feat: warn if some confused configurations exist in config #637

Merged
merged 2 commits into from
Mar 24, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
16 changes: 16 additions & 0 deletions config/config.default.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,22 @@ module.exports = appInfo => {
dump: {
ignore: new Set([ 'pass', 'pwd', 'passd', 'passwd', 'password', 'keys', 'secret' ]),
},

/**
* configurations are confused to users
* {
* [unexpectedKey]: [expectedKey],
* }
* @member Config#confusedConfigurations
* @type {Object}
*/
confusedConfigurations: {
bodyparser: 'bodyParser',
notFound: 'notfound',
Copy link
Member

Choose a reason for hiding this comment

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

命令规则好不一致

Copy link
Member

Choose a reason for hiding this comment

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

确实

Copy link
Member Author

Choose a reason for hiding this comment

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

是的,木已成舟。。

Copy link
Member

Choose a reason for hiding this comment

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

还好吧,统一格式,然后兼容下?

Copy link
Member Author

Choose a reason for hiding this comment

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

不想做兼容,以后插件多了也没法全统一的,就这样吧。

sitefile: 'siteFile',
middlewares: 'middleware',
httpClient: 'httpclient',
},
};

/**
Expand Down
16 changes: 16 additions & 0 deletions lib/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Application extends EggApplication {

// dump config after loaded, ensure all the dynamic modifications will be recorded
this.dumpConfig();
this.warnConfusedConfig();
this.bindEvents();
}

Expand Down Expand Up @@ -213,6 +214,21 @@ class Application extends EggApplication {
// expose server to support websocket
this.on('server', server => this.onServer(server));
}

/**
* warn when confused configurations are present
*
* @private
*/
warnConfusedConfig() {
const confusedConfigurations = this.config.confusedConfigurations;
Object.keys(confusedConfigurations).forEach(key => {
if (this.config[key] !== undefined) {
this.logger.warn('Unexpected config key `%s` exists, Please use `%s` instead.',
key, confusedConfigurations[key]);
}
});
}
}

module.exports = Application;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

exports.middlewares = [];
exports.bodyparser = {};
exports.sitefile = {};
exports.notFound = {};
exports.httpClient = {};
3 changes: 3 additions & 0 deletions test/fixtures/apps/confused-configuration/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "confused-configuration"
}
13 changes: 13 additions & 0 deletions test/lib/application.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,19 @@ describe('test/lib/application.test.js', () => {
});
});

describe('warn confused configurations', () => {
it('should warn if confused configurations exist', function* () {
const app = utils.app('apps/confused-configuration');
yield app.ready();
const logs = fs.readFileSync(utils.getFilepath('apps/confused-configuration/logs/confused-configuration/confused-configuration-web.log'), 'utf8');
assert(logs.match(/Unexpected config key `bodyparser` exists, Please use `bodyParser` instead\./));
assert(logs.match(/Unexpected config key `notFound` exists, Please use `notfound` instead\./));
assert(logs.match(/Unexpected config key `sitefile` exists, Please use `siteFile` instead\./));
assert(logs.match(/Unexpected config key `middlewares` exists, Please use `middleware` instead\./));
assert(logs.match(/Unexpected config key `httpClient` exists, Please use `httpclient` instead\./));
});
});

describe('test on apps/demo', () => {
let app;
before(() => {
Expand Down