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

docs: several small fixes/improvements/refactorings for the guides #21589

Closed
wants to merge 4 commits into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Jan 17, 2018

PR Checklist

PR Type

[x] Documentation content changes

What is the current behavior?

Some minor issues (typos, inconsistentcies, wrong/incomplete examples).

What is the new behavior?

Less minor issues.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Related: #21517, #21531

@gkalpak gkalpak added comp: docs action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Jan 17, 2018
Copy link
Contributor

@kapunahelewong kapunahelewong left a comment

Choose a reason for hiding this comment

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

This is excellent. Thank you so much, @gkalpak!!

@ocombe ocombe added comp: docs action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release and removed action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release comp: docs labels Jan 17, 2018
@gkalpak gkalpak closed this Jan 17, 2018
@gkalpak gkalpak reopened this Jan 17, 2018
@mary-poppins
Copy link

You can preview 2163fb1 at https://pr21589-2163fb1.ngbuilds.io/.

@@ -130,5 +126,3 @@ You may also be interested in the following:
* [Lazy Loading Modules with the Angular Router](guide/lazy-loading-ngmodules).
* [Providers](guide/providers).
* [Types of NgModules](guide/module-types).
Copy link
Member

Choose a reason for hiding this comment

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

Does the text for this link need to change to Types of Feature Modules ?

@@ -189,4 +189,3 @@ The following table summarizes the key characteristics of each feature module gr
You may also be interested in the following:
* [Lazy Loading Modules with the Angular Router](guide/lazy-loading-ngmodules).
* [Providers](guide/providers).
* [Types of NgModules](guide/module-types).
Copy link
Member

Choose a reason for hiding this comment

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

LOL - you might also be interested in "this page"!

<ol>
<li>When compiling a template, you need to determine a set of selectors which should be used for triggering their corresponding directives.</li>
<li>
The template is compiled within a context of an `NgModule`&mdash;the `NgModule` which this template's component is declared in&mdash;which determines the set of selectors using the following rules:
Copy link
Member

Choose a reason for hiding this comment

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

"within the context"?

Copy link
Member

Choose a reason for hiding this comment

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

"the NgModule within which the template's component is declared"

The template is compiled within a context of an `NgModule`&mdash;the `NgModule` which this template's component is declared in&mdash;which determines the set of selectors using the following rules:
<ul>
<li>All selectors of directives listed in `declarations`.</li>
<li>All exported selectors of imported `NgModules`.</li>
Copy link
Member

Choose a reason for hiding this comment

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

"All selectors of directives exported from imported NgModules"?


<tr>
A list of modules which should be folded into this module. Folded means it is
as if all of the imported NgModule properties were declared here.
Copy link
Member

Choose a reason for hiding this comment

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

"all the imported NgModule's exported properties"


## What should I export?

Export [declarable](guide/bootstrapping#the-declarations-array) classes that components in _other_ NgModules
are able to reference in their templates. These are your _public_ classes.
If you don't export a class, it stays _private_, visible only to other component
If you don't export a declarable class, it stays _private_, visible only to other component
Copy link
Member

Choose a reason for hiding this comment

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

other components

@@ -55,13 +55,13 @@ of some of the things they contain:

<tr>
<td><code>RouterModule</code></td>
<td><code>@angular/forms</code></td>
<td><code>@angular/router</code></td>
Copy link
Member

Choose a reason for hiding this comment

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

Oops :-)


<hr />

## Providing a singleton service

An injector created from a module definition will have services which are singletons with respect to that injector. To control the lifetime of services, one controls the creation and destruction of injectors. For example, a route will have an associated module. When the route is activated, an injector is created from that module as a child of the current injector. When you navigate away from the route, the injector is destroyed. This means that services declared in a route module will have a lifetime equal to that of the route. Similarly, services provided in an application module will have the same lifetime of the application, hence singleton.
An injector created from a module definition will have services which are singletons with respect to that injector. To control the lifetime of services, one controls the creation and destruction of injectors. For example, a route will have an associated module. When the route is activated, an injector is created from that module as a child of the current injector. When you navigate away from the route, the injector is destroyed. This means that services declared in a route module will have a lifetime equal to that of the route. Similarly, services provided in an application module will have the same lifetime of the application, hence singleton.
Copy link
Member

Choose a reason for hiding this comment

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

Surely this line should be broken into shorter lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wish all lines were limited to 100 chars :)

Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Some nits that I think would benefit this PR but nothing to really stop it from landing.

@petebacondarwin petebacondarwin added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 17, 2018
@mary-poppins
Copy link

You can preview d53e55c at https://pr21589-d53e55c.ngbuilds.io/.

@gkalpak gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jan 18, 2018
@gkalpak
Copy link
Member Author

gkalpak commented Jan 18, 2018

Incorporated @petebacondarwin's suggestions. ❤️
CI is green. ✨
Marked for merge. :shipit:

mhevery pushed a commit that referenced this pull request Jan 19, 2018
mhevery pushed a commit that referenced this pull request Jan 19, 2018
mhevery pushed a commit that referenced this pull request Jan 19, 2018
mhevery pushed a commit that referenced this pull request Jan 19, 2018
@alexeagle alexeagle closed this Jan 19, 2018
@gkalpak gkalpak deleted the docs-aio-fix-ngmodules branch January 19, 2018 09:50
mhevery pushed a commit that referenced this pull request Jan 19, 2018
mhevery pushed a commit that referenced this pull request Jan 19, 2018
mhevery pushed a commit that referenced this pull request Jan 19, 2018
mhevery pushed a commit that referenced this pull request Jan 19, 2018
jbogarthyde pushed a commit to jbogarthyde/angular that referenced this pull request Feb 23, 2018
jbogarthyde pushed a commit to jbogarthyde/angular that referenced this pull request Feb 23, 2018
jbogarthyde pushed a commit to jbogarthyde/angular that referenced this pull request Feb 23, 2018
jbogarthyde pushed a commit to jbogarthyde/angular that referenced this pull request Feb 23, 2018
leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018
leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018
leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018
leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018
@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 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants