Warn about options that are invalid for a given workbox-build mode #789
Conversation
| */ | ||
| module.exports = (blacklist, fullConfig) => { | ||
| const invalidConfig = Object.keys(fullConfig) | ||
| .filter((configKey) => blacklist.indexOf(configKey) !== -1); |
There was a problem hiding this comment.
Could you use includes?
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes
There was a problem hiding this comment.
We're still aiming for Node 4 compatibility with these modules, so no Array.prototype.includes(): http://node.green/#ES2016-features-Array-prototype-includes
| * @param {Object<String,String>} fullConfig The configuration options to check. | ||
| */ | ||
| module.exports = (blacklist, fullConfig) => { | ||
| const invalidConfig = Object.keys(fullConfig) |
There was a problem hiding this comment.
Nit: Rename to invalidConfigKeys (Just reads better for if (invalidConfigKeys.length > 0)
|
|
||
| if (invalidConfig.length > 0) { | ||
| logHelper.warn(`These Workbox configuration options are not valid for ` + | ||
| `the current mode, and have been ignored: ${invalidConfig}`); |
There was a problem hiding this comment.
Nit: Swap "have been" with "will be" as I'm assuming this log will be at the start of the build task (whatever that will be).
Would also make sense to reword this slightly away from "These Workbox configuration options". It's more to highlight that in this context it won't be used, so maybe:
The configuration options '${invalidConfig.join(',')}' will be ignored by 'workbox-build.${methodName}'.
There was a problem hiding this comment.
Folks might end up calling the methods indirectly via the Webpack or CLI, so I don't want to put workbox-build. in the message. But I'll reword along those lines.
R: @addyosmani @gauntface
Fixes #708
The decision reached in that issue was to log a warning, but not fail the build.