-
Notifications
You must be signed in to change notification settings - Fork 12k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Service worker in the CLI #4544
Conversation
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.
Few nits and questions, nothing major. Good work!
@@ -15,6 +15,7 @@ | |||
"index": "index.html", | |||
"main": "main.ts", | |||
"polyfills": "polyfills.ts", | |||
"serviceWorker": false, |
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.
Since the default is false, any reason to include this?
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.
Discoverability, really.
Removing it :)
packages/@angular/cli/package.json
Outdated
@@ -27,6 +27,7 @@ | |||
"homepage": "https://github.com/angular/angular-cli", | |||
"dependencies": { | |||
"@ngtools/json-schema": "^1.0.0", | |||
"@angular/service-worker": "1.0.0-beta.3", |
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.
Didn't we say this was an optional dep?
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.
Optional for the end user project.
@@ -25,6 +25,7 @@ | |||
"npm": ">= 3.0.0" | |||
}, | |||
"dependencies": { | |||
"@angular/service-worker": "1.0.0-beta.3", |
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.
This shouldn't be here, since the webpack plugin itself doesnt use the service worker.
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.
Agreed, I can remove it.
.then(() => ng('build', '--prod')) | ||
.then(() => expectFileToExist(join(process.cwd(), 'dist'))) | ||
.then(() => expectFileToExist(join(process.cwd(), 'dist/ngsw-manifest.json'))) | ||
.then(() => ng('set', 'apps.0.serviceWorker=false')); |
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.
No need to reset after a test, it's git clean
-ed.
… added before completing
ac2e319
to
55ac373
Compare
66f7bbc
to
694bf59
Compare
Adds the flag 'serviceWorker' to angular-cli.json that enables support for @angular/service-worker. When this flag is true, production builds will be set up with a service worker. A ngsw-manifest.json file will be generated (or augmented) in the dist/ root, and the service worker script will be copied there. A short script will be added to index.html to register the service worker. @angular/service-worker is a dependency of @angular/cli, but not of generated projects. It is desirable for users to be able to update the version of @angular/service-worker used in their apps independently of the CLI version. Thus, the CLI will error if serviceWorker=true but @angular/service-worker is not installed in the application's node_modules, as it pulls all the service worker scripts from there. If the flag is false the effect on the CLI is minimal - the webpack plugins associated with the SW are not even require()'d.
Adds the flag 'serviceWorker' to angular-cli.json that enables support for @angular/service-worker. When this flag is true, production builds will be set up with a service worker. A ngsw-manifest.json file will be generated (or augmented) in the dist/ root, and the service worker script will be copied there. A short script will be added to index.html to register the service worker. @angular/service-worker is a dependency of @angular/cli, but not of generated projects. It is desirable for users to be able to update the version of @angular/service-worker used in their apps independently of the CLI version. Thus, the CLI will error if serviceWorker=true but @angular/service-worker is not installed in the application's node_modules, as it pulls all the service worker scripts from there. If the flag is false the effect on the CLI is minimal - the webpack plugins associated with the SW are not even require()'d. Closes angular#4544
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.