Skip to content
This repository was archived by the owner on Dec 4, 2017. It is now read-only.

docs(toh): put a bit of clarification #1352

Merged
merged 1 commit into from
May 21, 2016
Merged

Conversation

Foxandxss
Copy link
Member

So @johnpapa got some feedback on the ToH and asked me to try to improve it.

Let me go step by step:

Multiple components

Let's add that array property to the bottom of the @component configuration object, immediately after the template and styles properties.

Which file?
Also, does it matter where in @component configuration this is put?

I put a label in the example to specify which file and also reworded the sentence a bit to tell that where to put the array is a matter of choice than a fixed place.

Let’s verify that we have the following structure

You might want to say “.js and .map files not shown

This part refers to the filetrees. I personally don't think that we need to explicitly say that the .js and .map files are not shown.


Services

The Injector has a container of previously created services. Either it finds and returns a pre-existing HeroService from its container or it creates a new instance, adds it to the container, and returns it to Angular.

Say a bit more about the container. Is this an actual object I could interact with?

After this bit, we have a link about "Learn more about dependency injection". Personally I don't think we can or even if we should talk more about containers in the tutorial. Truth being said, we never ever say the word "container" anymore in either "Dependency Injection" or "Hierarchical injectors" guides. Perhaps we could do some work on "Dependency Injection" to name those containers so the reader won't get confused.

Recall that the AppComponent creates an instance of HeroDetail by virtue of the tag at the bottom of its template. That HeroDetail is a child of the AppComponent.
If the HeroDetailComponent needed its parent component's HeroService, it would ask Angular to inject the service into its constructor which would look just like the one for AppComponent:

This may require a bit of rewriting into smaller conceptual bites.

constructor(private heroService: HeroService) { }

I don't think you told me to add the import statement first.

This is a blue box on the guide that explains that if we want to inject HeroService in the child component, we shouldn't use the providers array. What I did here is to update the code example label to say that that is an example. It is easy for the reader to get confused and think that the snippet is something that they should code. So yes, no import statement because that is just an example.

That being said, I think that we could get rid of the box. That is just like an excerpt of the Dependency Injection guide and we cannot rewrite it into smaller conceptual bites, because that is what the Dependency injection guide does. I suggest removing the blue box or making it more explicit that this is an excerpt of the Dependency Injection guide and that they can learn more there.

Appendix: Shadowing the parent's service

It isn’t clear what creates the parent-child relationship

This appendix is a continuation from the blue box and again, something that is already explained elsewhere and confuses the reader. I would drop it too.


So I updated the parts I see value in and if someone has a good feedback on the other points, I will update this PR.

`@Component` configuration object, immediately after the `template` and `styles` properties.
+makeExample('toh-3/ts/app/app.component.ts', 'directives')

We tell Angular about it by listing it in the metadata `directives` array. Let's add that array property to the `@Component` configuration object,
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the original wording, just with the name of the file in it.

@johnpapa
Copy link
Contributor

I don't see the changes to the "services" that you mention above. Where are they?

@Foxandxss
Copy link
Member Author

I don't see the changes to the "services" that you mention above. Where are they?

I just changed the excerpt to say that it is an example so it shouldn't be coded in the real app. Apart from that, I exposed there my personal opinion on what we should do, but didn't do the changes.

So the idea here is to agree on something so I can update this PR with those changes.

@johnpapa
Copy link
Contributor

I'm not sure I see a problem with leaving that part as it is. But if you feel strongly, let's get some other feedback too.

@Foxandxss
Copy link
Member Author

The person that gave the feedback things that the gray box (that explains hierarchical injection) is a bit confusing and should be explained more "step by step". I can understand that the gray box is a bit "complicated", but that gray box is just an excerpt of another guide we have, so instead of rewording the gray box, we can point people to the original guide.

There is also another part in services that talks about injections and containers and that person is confused about the word "containers". Containers is a DI concept but our DI guides never say "containers", that could be confusing.

@wardbell
Copy link
Contributor

wardbell commented May 19, 2016

@Foxandxss I agree with _all_ of your points including deleting all the complicated stuff about containers and what might happen if we did x, y, or z. Just link back to the pertinent DI chapter

Please make it so in this branch/PR and let me know when you want me to review it.

@Foxandxss
Copy link
Member Author

@wardbell Ready. I got rid of all those excerpt that tries to explain too much about DI and hierarchical DI and replaced them with links (or nothing for the appendix).

@wardbell wardbell merged commit 33b5829 into angular:master May 21, 2016
@wardbell wardbell deleted the toh-updates branch May 21, 2016 06:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants