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

Addressing issue #1862 by filtering out Boolean values from the plugins list #1927

Merged
merged 2 commits into from
Jun 13, 2017

Conversation

nitsakh
Copy link
Contributor

@nitsakh nitsakh commented Jun 12, 2017

This adds the .filter(Boolean) to filter out the undefined values in the config plugins array.

@@ -36,6 +36,8 @@ const _init = function (cfg) {
notify('Error reading configuration: `config` key is missing');
return _extractDefault(cfg.defaultCfg);
}
// Ignore undefined values in plugin array Issue #1862
_cfg.plugins = _cfg.plugins.filter(Boolean);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could _cfg.plugins be undefined? If yes we should check it:

_cfg.plugins = (_cfg.plugins && _cfg.plugins.filter(Boolean)) || [];

localPlugins should be filtered too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right! Will add that. About localPlugins I was thinking that it is dev only, so thought it's not needed to run through that check each time unnecessary.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If localPluginsis empty, it will do nothing. It is pretty useless you're right, but it is better for consistency imo.

Copy link
Contributor

@albinekb albinekb left a comment

Choose a reason for hiding this comment

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

👌 nice, the config file needs more validation, like check Array.isArray etc, but this will do for now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants