Closed rexxars closed 8 years ago
Sorry, I didn't test this under Node 0.12. Added some code to work around Object.assign
not being present there.
Hi @rexxars, thanks for the pull request!
Do we need the isPlainObject
checking? Any time that I've been using options objects, it's always been obvious that we should supply a plain object, that is pretty much a standard pattern across most if not all JS modules.
Also, I think it would be nice if you could remove the polyfill and let Babel polyfill it for 0.12, via object spread. So we can get rid of a lot of code by swapping out;
return assign({}, defaultOpts, specifiedOpts);
with
return {
...defaultOpts,
...specifiedOpts,
};
Also, defaultPluginOptions
is a little verbose. Perhaps simply options
is a better name for this?
Thanks for the feedback, @ben-eb
The reason why I added the object check was that there is already handling of non-object options in the codebase (postcss-discard-font-face can receive an array, for instance). Be happy to remove it if you think it's not necessary.
Updated PR based on your feedback, awaiting reply regarding object check.
Ah, OK. That makes sense to me. 👍
Merged and released on npm in v2.3.0. Thanks again! 🎉
As outlined in #11, it would be useful to be able to provide default options for plugins.
This implements that. I couldn't see any good way to merge non-object options, such as arrays - it's unclear whether it's meant to override or merge them when this is the case.