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

stop directly modifying the object passed to generateServiceWorker() #27

Merged
merged 1 commit into from
Aug 24, 2016

Conversation

FredKSchott
Copy link
Contributor

Fixes https://github.com/Polymer/polymer-cli/issues/345

This is much more serious than I thought, service-worker generation is currently broken in the CLI when a user defines their own swConfig.staticFileGlobs array.

/cc @justinfagnani Lets get this merged in ASAP along with the polymer.json CLI fix

let project = options.project;
let buildRoot = options.buildRoot;
let swConfig: SWConfig = options.swConfig || {};
let swConfig: SWConfig = Object.assign({}, options.swConfig);

return project.analyzer.analyzeDependencies.then((depsIndex: DepsIndex) => {
let staticFileGlobs = swConfig.staticFileGlobs || [];
Copy link
Contributor

Choose a reason for hiding this comment

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

to make sure, you might want to do similar here:

let staticFileGlobs = Array.from(swConfig.staticFileGlobs || []);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we actually never modify swConfig.staticFileGlobs. Instead we return new arrays via concat and then overwrite the staticFileGlobs property with one of those new arrays. So this isn't an issue today.

BUT you're right, code changes and this might not be the case forever. I'll add this extra precaution back in.

@justinfagnani
Copy link
Contributor

LGTM

@FredKSchott FredKSchott force-pushed the fix-service-worker-options-bug branch from b0b8eee to 914c453 Compare August 24, 2016 23:36
@FredKSchott FredKSchott merged commit 9eacff6 into master Aug 24, 2016
@FredKSchott FredKSchott deleted the fix-service-worker-options-bug branch August 24, 2016 23:36
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