-
Notifications
You must be signed in to change notification settings - Fork 388
Promote the use of module.exports for external configuration #198
Conversation
@@ -170,7 +170,8 @@ the service worker lifecycle event you can listen for to trigger this message. | |||
|
|||
For those who would prefer not to use `sw-precache` as part of a `gulp` or | |||
`Grunt` build, there's a [command-line interface](cli.js) which supports the | |||
[options listed](#options-parameter) in the API, provided via flags. | |||
[options listed](#options-parameter) in the API, provided via flags or an | |||
external JavaScript configuration file. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configuration via module.exports
LGTM. I'm glad we maintain a note about the --config JSON configuration option (I personally use this in a few projects).
Finally, there's support for storing a complex configuration as the | ||
[`module.exports`](https://nodejs.org/api/modules.html#modules_module_exports) | ||
of an external JavaScript file, using `--config <file>`. Any of the options from | ||
the file can be overridden via a command-line flag. For example, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggested text edits.
Finally, there's support for passing complex configurations using --config <file>
. Any of the options from the file can be overridden via a command-line flag. We strongly recommend passing it an external JavaScript file defining config via module.exports. For example: ...
This provides a great deal of flexibility, such as providing a regular expression for the runtimeCaching.urlPattern
option.
We also support passing in JSON for config, however this provides more limited flexibility. [Show an example of this too].
JavaScript file that defines `module.exports` allows for more flexibility than | ||
JSON allows, such as providing a regular expression for the | ||
runtimeCaching.urlPattern` option.) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you end up going for the earlier suggestions this text may end up getting moved around a little, but that's totally your call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per earlier, I'd suggest keeping in an example of the JSON config that is supported. As long as a feature is supported, it's good to show how it can be used imo (if this gets dropped at a later point then discouraging by not including examples makes more sense).
Thanks, @addyosmani. PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
R: @addyosmani @gauntface
This is a change to the documentation and the sample configuration file to encourage folks to use
module.exports
and a JavaScript object to configuration the command-line behavior.This didn't entail a code change, since
require()
was already being used inside the command-line interface to read in the JSON configuration.In addition to generally having a nicer syntax, encouraging the use of JavaScript configuration means that you can easily pass in a
RegExp
literal toruntimeCaching.urlPattern
, which wasn't possible when using JSON.Fixes #195