docs: fix bug in code examples for downgrading a service #19371

bertyhell · 2017-09-25T10:22:54Z

No description provided.

gkalpak · 2017-09-25T10:38:23Z

The service is indeed called heroes in AngularJS-land. This change seems invalid.

bertyhell · 2017-09-25T12:01:35Z

The service is indeed called heroes in AngularJS-land. This change seems invalid.

If that is the case, the documentation is even more confusing, since it states the opposite here:
https://angular.io/guide/upgrade#making-angularjs-dependencies-injectable-to-angular

In these situations, it is possible to upgrade an AngularJS provider to Angular. This makes it possible to then inject it somewhere in Angular code. For example, you might have a service called HeroesService in AngularJS:

gkalpak · 2017-09-25T14:54:01Z

It is indeed somewhat confusing, because we never show how the service is registered in AngularJS. In both Angular and AngularJS there are two things that identify a registered service:

The injection token, i.e. an identifier that we will use to request the service from the injector.
The service class (which will be used by the injector to create an instance of the service).

In Angular, it is common to use the class itself as the injection token:

providers: [
  {provide: MyClass, useClass: MyClass},
  // or the shorthand:
  MyClass
]

This is possible, because Angular's injection system can accept any token as identifier.

In AngularJS on the other hand, we only accept string identifiers as token. Thus you are forced to use two different things: a string (for the injection token) and a class (for the service)

.service('myServiceId', MyServiceClass)

It is common in AngularJS apps to chose a service name for the token (e.g. user) and append the Service suffix to create the class name:

.service('user', class UserService { ... })

Hopefully that clears it up a bit. Feel free to update to clarify things (as long as the change is valid 😃).

bertyhell · 2017-09-25T15:15:57Z

Ok, I changed the token name back to the original and added a note to explain the 'heroes' string.
Does that work for you?

https://github.com/angular/angular/pull/19371/files#diff-33c8eb0a067223c3fbf3dd45ec689efdR783

trotyl · 2017-09-25T15:20:18Z

This is possible, because Angular's injection system can accept any token as identifier.

Hi @gkalpak , sorry for off topic, I guess there're some different voices on #15319 (comment), may I know what is the expected validity for DI token now (or planned future)?

bertyhell · 2017-09-25T15:34:06Z

@trotyl Good point, the question is: After the 5.0 release will people be upgrading their app from 1.x => 4.x or 1.x => 5.x.

Either way, the docs will have to be updated to reflect this, but that's beyond the scope of this PR.

gkalpak · 2017-09-25T16:48:55Z

@trotyl, yes, things might change in future version. I am not sure what the current state is.
@bertyhell, what we have in the docs now will work with 4.x and 5.x alike, since we are using a Type (HeroesService) as token (in this case at least).

gkalpak · 2017-09-25T19:14:09Z

aio/content/examples/upgrade-module/src/app/ajs-to-a-providers/hero-detail.component.ts

@@ -5,6 +5,7 @@ import { Hero } from '../hero';

 @Component({
  selector: 'hero-detail',
+  providers: [HeroesService],


Why this change?

@gkalpak Without this change I get this error in the app I'm trying to migrate:

ERROR Error: Uncaught (in promise): Error: [$injector:unpr] Unknown provider: heroesServiceProvider <- heroesService

This example provides the service on the NgModule. Providing it on the component too, will create a separate instance for the component (different than what the rest of the app is using). This is not what we want here (but might be what you want in your app).

To share the same instance of the service across components, put it on the providers array of the NgModule.

bertyhell · 2017-10-02T11:58:22Z

To share the same instance of the service across components, put it on the providers array of the NgModule.

I modified the code and added some more description text to the readme.

kapunahelewong

Thanks for the updates @bertyhell. Just a couple of comments for you.

kapunahelewong · 2017-10-17T18:23:45Z

aio/content/guide/upgrade.md

+
+Note that the 'heroes' string inside the factory refers to the AngularJS `HeroesService`.
+It is common in AngularJS apps to chose a service name for the token (e.g. heroes)
+and append the Service suffix to create the class name (e.g. heroesService)


Could you please change "class name (e.g. heroesService)" to "class name, for example heroesService"? The reasons for changes are:

We are trying to minimize parentheticals whenever we can.

We're replacing i.e. and e.g. with "that is" and "for example" since people often confuse them.

For consistency, heroesService should be in code font, thus the back ticks.

kapunahelewong · 2017-10-17T18:24:36Z

aio/content/guide/upgrade.md

 <code-example path="upgrade-module/src/app/ajs-to-a-providers/app.module.ts" region="register" title="app.module.ts">
 </code-example>

-You can then inject it in Angular using it's class as a type annotation:
+Then use the service inside your component by injecting it in the component constructor using it's class as a type annotation:


it's --> its

I know this was already there, but since you're on the line, would you mind deleting the apostrophe?

bertyhell · 2017-12-04T13:43:23Z

@kapunahelewong Sorry for the long delay. I missed the email notification about your comments.

ngbot · 2018-01-30T20:06:18Z

Hi @bertyhell! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

ngbot · 2018-01-30T20:07:32Z

Hi @bertyhell! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

kapunahelewong · 2018-02-23T18:18:22Z

Hey @bertyhell, if you'd like to rebase, I'd be happy to approve :)

EDIT: oh, and squash.

brandonroberts · 2018-08-06T14:13:49Z

@gkalpak are there any changes left on this from your point of view?

brandonroberts · 2018-08-06T14:14:14Z

aio/content/guide/upgrade.md

@@ -778,16 +778,25 @@ delete them once the upgrade is over.
 It's also recommended to export the `heroesServiceFactory` function so that Ahead-of-Time
 compilation can pick it up.

+<div class="l-sub-section">


Change l-sub-section to alert is-helpful

brandonroberts

Minor css change

gkalpak

a couple of minor comments. LGTM otherwise.

gkalpak · 2018-08-06T14:24:26Z

aio/content/guide/upgrade.md

+<div class="l-sub-section">
+
+Note that the 'heroes' string inside the factory refers to the AngularJS `HeroesService`.
+It is common in AngularJS apps to chose a service name for the token, for example heroes,


chose --> choose
for example heroes --> for example "heroes"

gkalpak · 2018-08-06T14:25:39Z

aio/content/guide/upgrade.md

+
+Note that the 'heroes' string inside the factory refers to the AngularJS `HeroesService`.
+It is common in AngularJS apps to chose a service name for the token, for example heroes,
+and append the Service suffix to create the class name, for example `heroesService`.


Service --> "Service"
Also, classes (or constructor functions) start with an uppercase letter (by convention). So HeroesService, not heroesService.

gkalpak · 2018-08-06T14:26:10Z

aio/content/guide/upgrade.md

 <code-example path="upgrade-module/src/app/ajs-to-a-providers/ajs-upgraded-providers.ts" title="ajs-upgraded-providers.ts">
 </code-example>

+You can then add the service to Angular by adding it to the `@NgModule`:


I would change "add the service" to "provide the service".

brandonroberts · 2018-08-16T21:00:39Z

@gkalpak this one is ready for another review

gkalpak

#19371 (comment) hasn't been fully addressed. Other than that LGTM.

PR Close #19371

PR Close angular#19371

angular-automatic-lock-bot · 2019-09-13T23:51:37Z

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 25, 2017

gkalpak added comp: docs labels Sep 25, 2017

bertyhell force-pushed the fix-downgrade-service-docs branch from 4c50615 to e59eb37 Compare September 25, 2017 15:29

gkalpak reviewed Sep 25, 2017

View reviewed changes

kapunahelewong suggested changes Oct 17, 2017

View reviewed changes

IgorMinar removed the type: docs label Apr 10, 2018

bertyhell force-pushed the fix-downgrade-service-docs branch from 77495e0 to 9b32232 Compare May 23, 2018 08:29

jenniferfell added state: community Someone from the Angular community is working on this issue or submitted this PR and removed state: community Someone from the Angular community is working on this issue or submitted this PR labels Jul 23, 2018

brandonroberts added target: patch This PR is targeted for the next patch release docsarea: upgrading-AJS labels Aug 6, 2018

brandonroberts reviewed Aug 6, 2018

View reviewed changes

brandonroberts suggested changes Aug 6, 2018

View reviewed changes

gkalpak reviewed Aug 6, 2018

View reviewed changes

brandonroberts force-pushed the fix-downgrade-service-docs branch from 9b32232 to b8a4277 Compare August 16, 2018 20:59

brandonroberts self-assigned this Aug 16, 2018

brandonroberts changed the title ~~docs(upgrade): Fix bug in code examples for downgrading a service~~ docs: fix bug in code examples for downgrading a service Aug 16, 2018

gkalpak approved these changes Aug 20, 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 Aug 20, 2018

brandonroberts force-pushed the fix-downgrade-service-docs branch from b8a4277 to 99807c4 Compare August 20, 2018 13:04

docs: Improve docs for downgrading a service

c3997c9

brandonroberts force-pushed the fix-downgrade-service-docs branch from 99807c4 to c3997c9 Compare August 20, 2018 13:10

brandonroberts approved these changes Aug 20, 2018

View reviewed changes

brandonroberts removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Aug 20, 2018

kapunahelewong approved these changes Aug 21, 2018

View reviewed changes

brandonroberts added the action: merge The PR is ready for merge by the caretaker label Aug 21, 2018

jasonaden closed this in 69b57b2 Aug 21, 2018

jasonaden pushed a commit that referenced this pull request Aug 21, 2018

docs: Improve docs for downgrading a service (#19371)

1fa9790

PR Close #19371

petebacondarwin pushed a commit to petebacondarwin/angular that referenced this pull request Aug 22, 2018

docs: Improve docs for downgrading a service (angular#19371)

0d2ffb5

PR Close angular#19371

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

docs: Improve docs for downgrading a service (angular#19371)

00c5fed

PR Close angular#19371

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: fix bug in code examples for downgrading a service #19371

docs: fix bug in code examples for downgrading a service #19371

bertyhell commented Sep 25, 2017

gkalpak commented Sep 25, 2017

bertyhell commented Sep 25, 2017

gkalpak commented Sep 25, 2017

bertyhell commented Sep 25, 2017

trotyl commented Sep 25, 2017

bertyhell commented Sep 25, 2017

gkalpak commented Sep 25, 2017

gkalpak Sep 25, 2017

bertyhell Sep 26, 2017 •

edited

gkalpak Sep 26, 2017

bertyhell commented Oct 2, 2017

kapunahelewong left a comment

kapunahelewong Oct 17, 2017

kapunahelewong Oct 17, 2017

bertyhell commented Dec 4, 2017

ngbot bot commented Jan 30, 2018

ngbot bot commented Jan 30, 2018

kapunahelewong commented Feb 23, 2018 •

edited

brandonroberts commented Aug 6, 2018

brandonroberts Aug 6, 2018

brandonroberts left a comment

gkalpak left a comment

gkalpak Aug 6, 2018

gkalpak Aug 6, 2018

gkalpak Aug 6, 2018

brandonroberts commented Aug 16, 2018

gkalpak left a comment

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

docs: fix bug in code examples for downgrading a service #19371

docs: fix bug in code examples for downgrading a service #19371

Conversation

bertyhell commented Sep 25, 2017

gkalpak commented Sep 25, 2017

bertyhell commented Sep 25, 2017

gkalpak commented Sep 25, 2017

bertyhell commented Sep 25, 2017

trotyl commented Sep 25, 2017

bertyhell commented Sep 25, 2017

gkalpak commented Sep 25, 2017

Choose a reason for hiding this comment

bertyhell Sep 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bertyhell commented Oct 2, 2017

kapunahelewong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bertyhell commented Dec 4, 2017

ngbot bot commented Jan 30, 2018

ngbot bot commented Jan 30, 2018

kapunahelewong commented Feb 23, 2018 • edited

brandonroberts commented Aug 6, 2018

Choose a reason for hiding this comment

brandonroberts left a comment

Choose a reason for hiding this comment

gkalpak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandonroberts commented Aug 16, 2018

gkalpak left a comment

Choose a reason for hiding this comment

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

bertyhell Sep 26, 2017 •

edited

kapunahelewong commented Feb 23, 2018 •

edited