Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

add service worker flag to build command #534

Merged
merged 6 commits into from
Jan 13, 2017

Conversation

FredKSchott
Copy link
Contributor

(CHANGELOG will be updated when new-build-flags is merged into master).

This PR adds a flag to the build command to generate a service worker. Like all the other new build flags this one is "default off" for now while we work towards the greater polymer.json build flags / build permutations story.

/cc @justinfagnani

Copy link
Contributor

@justinfagnani justinfagnani left a comment

Choose a reason for hiding this comment

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

I'm not sure that name changes away from sw-precache to more generic make sense if we don't actually support more generic service worker generation.

@@ -39,10 +39,19 @@ export class BuildCommand implements Command {
'requests needed to serve your application.'
},
{
name: 'sw-precache-config',
Copy link
Contributor

Choose a reason for hiding this comment

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

why rename this flag? The file is still sw-precache specific?

defaultValue: 'sw-precache-config.js',
description: 'Path to an sw-precache configuration to be ' +
'used for service worker generation.'
name: 'add-service-worker',
Copy link
Contributor

Choose a reason for hiding this comment

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

until we support more than sw-precache, we may want to name this something sw-precache specific. maybe --add-sw-precache or --use-sw-precache

@FredKSchott
Copy link
Contributor Author

@justinfagnani the thinking was to not tie the service worker generation to the sw-precache implementation directly. Instead the user just knows that they're getting a service worker, and doesn't need to know what sw-precache is or how it works until they want to configure it. This felt like a significant enough usability improvement to warrant the change.

For the config filepath flag I could go either way. But given the --add-service-worker flag, keeping the service-worker name in the config flag as well felt like a better fit.

@justinfagnani
Copy link
Contributor

I don't think we can uncouple the config from sw-precache because they have to know how to configure sw-precache from the config. The new flags docs even reference sw-precache docs.

@FredKSchott
Copy link
Contributor Author

bundled: options.bundle,
});
if (options.addServiceWorker) {
logger.debug(`Generating service worker...`);
Copy link
Contributor

Choose a reason for hiding this comment

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

I could see this as a valuable logger.info

@FredKSchott FredKSchott merged commit 7f9f478 into new-build-flags Jan 13, 2017
@FredKSchott FredKSchott deleted the add-service-worker-flag branch January 13, 2017 00:17
FredKSchott added a commit that referenced this pull request Jan 31, 2017
* Convert build to single build stream (#509)

* Convert build to single build stream, add bundled flag

* Clarify `--add-prefetch` build flag (#510)

* rename confusing --insert-dependency-links flag to --prefetch-dependencies

* rename prefetchDepedencies to addPrefetchLinks

* rename to insert-prefetch-links

* respond to review comments

* Cleanup how optimize streams work, in preparation for babel stream (#524)

* Cleanup how optimize streams work, in preparation for babel stream

* death to lets

* redeisn optimize interface

* pull stream piping into a helper function

* rollback cli dash-case naming

* remove unused gulpif

* update buildoptions to match planned polymer.json options

* add babel to build stream, replace uglify with babili es2015 minifier (#525)

* Cleanup how optimize streams work, in preparation for babel stream

* add babel to build stream

* death to lets

* redeisn optimize interface

* pull stream piping into a helper function

* rollback cli dash-case naming

* replace uglify with babili

* fix bad merge conflict

* fixup js-minify description

* remove unused gulpif

* update buildoptions to match planned polymer.json options

* remove old comment

* let to const

* add service worker flag to build command (#534)

* add service worker flag to build command

* reword flags and descriptions

* quick reword again

* get rid of skiplibcheck

* debug to info

* Add support for polymer.json build configurations (#545)

* Add support for named build permutations

* respond to PR feedback

* add comments on build function

* update logging to be cleaner

* update for new naming rules

* update polymer-project-config to v1.2.0

* Update CHANGELOG.md

* update polymer-build version, update shrinkwrap
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants