Skip to content
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

Tour of Heroes tutorial, chapter 6 (services) - inclusion of HeroesService includes MessageService, which doesn't come until further down #20398

Closed
williamBurgeson opened this issue Nov 13, 2017 · 11 comments

Comments

@williamBurgeson
Copy link

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Feature request
[ x] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Having added the heroes service via the CLI, the copy/paste code invites us to add the providers, but includes the MessageService, which we don't see until later. This will confuse learners and is probably just an error.

providers: [ HeroService, MessageService ],

It would probably also be worth mentioning that there is already an empty providers there, which should be overwritten.

A simple error which should be relatively easy to fix but would make life a lot easier for newbies trying to follow.

@gkalpak
Copy link
Member

gkalpak commented Nov 13, 2017

Good catch!

@JB711
Copy link

JB711 commented Dec 9, 2017

running into the same issue...

jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Dec 31, 2017
…e, which doesn't come until further down (angular#20398)

Change TOH docs where the MessageService is referenced within the Providers array, but it isn't generated until later in the Services tutorial.
Fixes angular#20398
@4E71-NOP
Copy link

4E71-NOP commented Jan 6, 2018

That helped fixing it. Thx @jhenderson2099

On the other hand this tutorial could benefit from a global description of "what we need" and "how it works" as an introduction. A diagram could help too (a 1000 words vs 1 image).

Or split it in 2 or more parts. At the end of each part the application should still work.

This document is rather long.

I mean by that :
-7 files are listed in the final code review. Not to mention the syntax errors we all do (or our IDE auto completion sneaky attack)... It's not a light change for this "hero" application.
-I was tired and i didn't know what i was doing (the goal) and how to achieve it (the means) approximatively at half of this document. I was wondering when does it ends?

I don't think opening another issue is relevant for this (no category that fit). But in the end it may help the authors to make this tutorial evolve.

(Frenchie' over here, sorry for my English)

@jhenderson2099
Copy link
Contributor

No problem @4E71-NOP

I agree. The application should work each time the instructions indicate to "try it".

Although, I can hardly take credit. I'm just the person who submitted a fix in the documentation and submitted the pull request. Hopefully it will be incorporated into the online documentation soon :-D

jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Jan 13, 2018
@ScottMcCormack
Copy link

I believe the error in the documentation comes from the code referenced at this location:

providers: [ HeroService, MessageService ],

The md file that references this module is located here:
https://github.com/angular/angular/blob/456f57dde804733f1c41bd3fba477e7f58fd8399/aio/content/tutorial/toh-pt4.md

@jhenderson2099
Copy link
Contributor

This has been fixed with a documentation patch that I submitted (above). Awaiting approval for pull request.

jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Jan 23, 2018
Change docs where the MessageService is referenced
Fixes angular#20398
jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Jan 23, 2018
Change docs where the MessageService is referenced
Fixes angular#20398
jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Jan 23, 2018
Change docs where the MessageService is referenced
Fixes angular#20398
jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Jan 23, 2018
Change docs where the MessageService is referenced
Fixes angular#20398
jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Jan 25, 2018
Change MessageService reference/formatting
Fixes angular#20398
@maxpown3r
Copy link

Hi, This error has not been fixed. Newbie here that spend an hour depbugging before commenting out "MessageService"

jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Feb 4, 2018
Change docs where the MessageService is referenced
Fixes angular#20398
jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Feb 4, 2018
Change docs where the MessageService is referenced
Fixes angular#20398
jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Feb 4, 2018
Change docs where the MessageService is referenced
Fixes angular#20398
jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Feb 4, 2018
Change docs where the MessageService is referenced
Fixes angular#20398
@jhenderson2099
Copy link
Contributor

Like I said...waiting on the pull request.

jhenderson2099 added a commit to jhenderson2099/angular that referenced this issue Feb 6, 2018
Change docs where the MessageService is referenced

Fixes angular#20398
@mhevery mhevery closed this as completed in 545fdf1 Feb 7, 2018
mhevery pushed a commit that referenced this issue Feb 7, 2018
Change docs where the MessageService is referenced

Fixes #20398

PR Close #21228
jbogarthyde pushed a commit to jbogarthyde/angular that referenced this issue Feb 23, 2018
Change docs where the MessageService is referenced

Fixes angular#20398

PR Close angular#21228
leo6104 pushed a commit to leo6104/angular that referenced this issue Mar 25, 2018
Change docs where the MessageService is referenced

Fixes angular#20398

PR Close angular#21228
@cgcerro
Copy link

cgcerro commented Oct 24, 2018

I'm following the tutorial and inclusion of the services in providers array is not needed!!!

But, i don't know whay, but in the MessageService you must change the name of the method "add" with otherone name like "insert". With "add" not compile.

MessageService:

import { Injectable } from '@angular/core';

@Injectable({
  providedIn: 'root'
})
export class MessageService {

  messages: string[] = [];

  insert(message: string) {
    this.messages.push(message);
  }

  clear() {
    this.messages = [];
  }

  constructor() { }
}

HeroService:

import { Injectable } from '@angular/core';
import { Hero } from './hero';
import { HEROES } from './mock-heroes';
import { Observable, of } from 'rxjs';
import { MessageService } from './message.service';

@Injectable({
  providedIn: 'root'
})
export class HeroService {

  getHeroes(): Observable<Hero[]> {
    this.addMessage();
    return of(HEROES);
  }

  constructor(private messageService: MessageService) {
    
   }

  private addMessage() {
    this.messageService.insert('fetched heroes');
  }
}

@williamBurgeson
Copy link
Author

Re the last comment, this is something which changed in ng 6 - the @Injectable decorator now has a providedIn property which, as you can see in the example, automatically has the effect of including it in the providers array of the app module.
That arg can also be the child module if appropriate, in such a case you need to refer to the module directly, rather than as a string

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants