feat(service-worker): expose RegistrationOptions token to allow for runtime configuration by deebloo · Pull Request #26002 · angular/angular

deebloo · 2018-09-18T18:17:58Z

Expose the RegistrationOptions DI token to allow for runtime configuration of ServiceWorkerModule. At the moment there is no way to mark ServiceWorkerModule as enabled at runtime since the configuration values are set at build time.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

At the moment there is no way to mark the sw module as enabled at runtime. For example if you use global variables for environment variables.

@NgModule({
  imports: [
    // this does not work since the configuration happens at compile time.
    ServiceWorkerModule.register('/sw/script.js', { enabled: window['isProdMode'] })
  ]
])
class AppModule {}

Issue Number: #22796

What is the new behavior?

Exposing this token will allow folks to configure the sw with useFactory making runtime config possible.

export function swConfigFactory() {
  return { enabled: window['isProdMode'] }
}

@NgModule({
  imports: [ServiceWorkerModule.register('/sw/script.js')],
  providers: [
    { provide: RegistrationOptions, useFactory: swConfigFactory }
  ]
])
class AppModule {}

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

gkalpak · 2018-09-18T20:42:05Z

Related to #22796.

deebloo · 2018-09-18T20:58:45Z

must not have a good job looking for an issue. I noticed that the test is failing because of public API restrictions. I was wondering the correct way going about updating those type def files. whether by hand or some tool?

gkalpak · 2018-09-19T06:36:23Z

There is bazel command you can run to update the "golden files". It is mentioned in the CI logs:

bazel run //tools/public_api_guard:service-worker_service-worker_api_bin.accept

(But for small changes it is usually easier to manually update the corresponding file, service-worker.d.ts in this case.)

gkalpak

What is the motivation for exporting this? Can you add it to the commit message (and PR description)?

If we are going to do it, then we at least need some docs 😉

deebloo · 2018-09-19T14:11:29Z

@gkalpak i updated the PR and the public_api_guard for service-worker. Let me know if anything needs to be clarified

gkalpak · 2018-09-19T15:48:01Z

#26002 (review)

deebloo · 2018-09-20T14:35:06Z

@gkalpak I believe I have everything in order. If not then I am missing the point and need more specific direction to penetrate my thick skull 😆

gkalpak · 2018-09-21T14:12:07Z

@deebloo, can you give an example of a usecase that was not possible before and is possible with this change?

deebloo · 2018-09-21T14:54:53Z

yes. ill update the PR to show what could NOT be done before

deebloo · 2018-09-21T15:05:50Z

done. updated with current behavior and better example for new behavior

mary-poppins · 2018-09-24T07:51:57Z

You can preview 8632c5d at https://pr26002-8632c5d.ngbuilds.io/.
You can preview 15e19bf at https://pr26002-15e19bf.ngbuilds.io/.
You can preview e6e12d6 at https://pr26002-e6e12d6.ngbuilds.io/.

gkalpak

Thx for the example. What we need:

Squash the commits into one (that follows our guidelines).
Add some API docs for RegistrationOptions (see #23138 for example).

It would be rad (but not necessary for this PR) if you wanted to add an example that shows how to use this feature (and has tests to ensure it doesn't break in the future). Let me know if you are interested 😉

mary-poppins · 2018-09-24T13:26:48Z

You can preview 3308537 at https://pr26002-3308537.ngbuilds.io/.

deebloo · 2018-09-25T02:39:53Z

@gkalpak id be happy to add

mary-poppins · 2018-09-25T02:46:40Z

You can preview a03f9e4 at https://pr26002-a03f9e4.ngbuilds.io/.

deebloo · 2018-09-25T16:17:55Z

@gkalpak I updated the unit tests but not really sure the best way to add an example. Let me know if you think this is good enough

mary-poppins · 2018-09-25T16:23:53Z

You can preview 40ab248 at https://pr26002-40ab248.ngbuilds.io/.

…untime config Previously, the ServiceWorker registration options should be defined as an object literal (in order for them to be compatible with Ahead-of-Time compilation), thus making it impossible to base the ServiceWorker behavior on runtime conditions. This commit allows specifying the registration options using a regular provider, which means that it can take advantage of the `useFactory` option to determine the config at runtime, while still remaining compatible with AoT compilation.

…o separate file

…ample

deebloo · 2019-03-27T14:09:26Z

oh cool thanks @gkalpak ill keep and eye on the PR so I can see what is different

gkalpak · 2019-03-27T14:18:15Z

@deebloo, I think it's ready. Please, take a look and let me know if you spot anything dubious 😃

More or less here is what I have done:

Rebased on master.
Added a commit (15b8cf4) to rename RegistrationOptions to SwRegistrationOptions (as suggested in feat(service-worker): add option to config when to register sw #21842 (review)).
Used SwRegistrationOptions in ServiceWorkerModule.register() (as suggested in feat(service-worker): add option to config when to register sw #21842 (review)).
Removed unnecessary commits (for config files that have since been removed).
Adapted the example to use the new bazel-based setup (1f8c38d).

mary-poppins · 2019-03-27T14:19:59Z

You can preview 1f8c38d at https://pr26002-1f8c38d.ngbuilds.io/.

deebloo · 2019-03-27T15:18:11Z

@gkalpak everything looks like it makes sense. I appreciate all the help getting this done.

gkalpak · 2019-03-27T23:28:41Z

@IgorMinar, PTAL.

glebmachine · 2019-04-17T16:44:19Z

Please, publish

It's very important due to #26500 bug!
Service worker in Safari is totally broken

IgorMinar

lgtm! thanks for getting this into a mergeable state and iterating on the PR

@gkalpak thanks for helping out! ❤️

googlebot · 2019-04-25T05:43:22Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

gkalpak · 2019-04-25T13:44:22Z

Thx for working in this, @deebloo ❤️
I am going to close this and include the commit in #21842 (per #21842 (review)).

deebloo · 2019-04-26T01:28:31Z

Of course! I do really appreciate all the assistance.

znikola · 2019-05-14T17:56:58Z

Hi, is this going to be in Angular 7 as well?

Splaktar · 2019-05-14T18:44:42Z

@znikola I don't believe so.

angular-automatic-lock-bot · 2019-09-15T01:53:18Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Sep 18, 2018

gkalpak suggested changes Sep 19, 2018

View reviewed changes

deebloo changed the title ~~feat(service-worker): expose RegistrationOptions token~~ feat(service-worker): expose RegistrationOptions token to allow for runtime configuration Sep 19, 2018

gkalpak added the aio: preview label Sep 24, 2018

gkalpak approved these changes Sep 24, 2018

View reviewed changes

gkalpak added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Sep 24, 2018

deebloo force-pushed the expose-sw-registration-options branch from e6e12d6 to 3308537 Compare September 24, 2018 13:18

deebloo force-pushed the expose-sw-registration-options branch from 3308537 to a03f9e4 Compare September 25, 2018 02:38

deebloo force-pushed the expose-sw-registration-options branch from a03f9e4 to 40ab248 Compare September 25, 2018 16:16

deebloo force-pushed the expose-sw-registration-options branch from 40ab248 to 89c5338 Compare September 25, 2018 18:31

deebloo and others added 3 commits March 27, 2019 15:06

test(service-worker): expand SwRegistrationOptions tests and move t…

ce1d9b4

…o separate file

docs(service-worker): improve SwRegistrationOptions docs and add ex…

1f8c38d

…ample

gkalpak force-pushed the expose-sw-registration-options branch from e217161 to 1f8c38d Compare March 27, 2019 14:08

deebloo requested a review from a team March 27, 2019 14:08

gkalpak approved these changes Mar 27, 2019

View reviewed changes

gkalpak requested a review from IgorMinar March 27, 2019 14:22

IgorMinar approved these changes Apr 25, 2019

View reviewed changes

IgorMinar added cla: yes and removed cla: no labels Apr 25, 2019

IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 25, 2019

IgorMinar modified the milestones: needsTriage, version 8 Apr 25, 2019

IgorMinar added the hotlist: release-blocker label Apr 25, 2019

gkalpak closed this Apr 25, 2019

trycatchal mentioned this pull request May 3, 2019

Add support for Angular Universal ngx-rocket/generator-ngx-rocket#230

Open

5 tasks

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 15, 2019

Conversation

deebloo commented Sep 18, 2018 • edited by gkalpak Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

gkalpak commented Sep 18, 2018

Uh oh!

deebloo commented Sep 18, 2018

Uh oh!

gkalpak commented Sep 19, 2018

Uh oh!

gkalpak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deebloo commented Sep 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gkalpak commented Sep 19, 2018

Uh oh!

deebloo commented Sep 20, 2018

Uh oh!

gkalpak commented Sep 21, 2018

Uh oh!

deebloo commented Sep 21, 2018

Uh oh!

deebloo commented Sep 21, 2018

Uh oh!

mary-poppins commented Sep 24, 2018

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

mary-poppins commented Sep 24, 2018

Uh oh!

deebloo commented Sep 25, 2018

Uh oh!

mary-poppins commented Sep 25, 2018

Uh oh!

deebloo commented Sep 25, 2018

Uh oh!

mary-poppins commented Sep 25, 2018

Uh oh!

deebloo commented Mar 27, 2019

Uh oh!

gkalpak commented Mar 27, 2019

Uh oh!

mary-poppins commented Mar 27, 2019

Uh oh!

deebloo commented Mar 27, 2019

Uh oh!

gkalpak commented Mar 27, 2019

Uh oh!

glebmachine commented Apr 17, 2019

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

googlebot commented Apr 25, 2019

Uh oh!

gkalpak commented Apr 25, 2019

Uh oh!

deebloo commented Apr 26, 2019

Uh oh!

znikola commented May 14, 2019

Uh oh!

Splaktar commented May 14, 2019

Uh oh!

angular-automatic-lock-bot bot commented Sep 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

deebloo commented Sep 18, 2018 •

edited by gkalpak

Loading

gkalpak left a comment •

edited

Loading

deebloo commented Sep 19, 2018 •

edited

Loading