docs(aio): update architecture section by jbogarthyde · Pull Request #21569 · angular/angular

jbogarthyde · 2018-01-16T20:30:13Z

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
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Update doc with new section on architecture.

jbogarthyde · 2018-01-16T20:34:52Z

@StephenFluin - The old PR was hopelessly munged. Please review this new PR with the changes we discussed, and some of the restored content that was lost in the previous PR.

googlebot · 2018-01-16T21:51:04Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

mary-poppins · 2018-01-17T10:48:12Z

You can preview a324941 at https://pr21569-a324941.ngbuilds.io/.

mary-poppins · 2018-01-17T17:27:34Z

You can preview d720ba1 at https://pr21569-d720ba1.ngbuilds.io/.

gkalpak · 2018-01-26T09:19:03Z

@jbogarthyde, can you please squash all commits into one and rebase on master?

mary-poppins · 2018-01-26T17:20:28Z

You can preview fdc0d52 at https://pr21569-fdc0d52.ngbuilds.io/.

jenniferfell · 2018-02-01T23:24:08Z

(wrong PR)

jenniferfell · 2018-02-02T01:01:33Z

I believe the rewrites in this PR also fix #21957 and #21955

ngbot · 2018-02-05T21:42:47Z

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

ngbot · 2018-02-05T21:50:31Z

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

mary-poppins · 2018-02-06T16:20:59Z

You can preview 4e9e77f at https://pr21569-4e9e77f.ngbuilds.io/.

mary-poppins · 2018-02-06T20:14:07Z

You can preview ffa43f0 at https://pr21569-ffa43f0.ngbuilds.io/.

wimgoeman · 2018-02-08T07:58:28Z

I don't think #21957 is fixed with this. aio/content/guide/architecture-components.md still references <hero-detail> while the html in aio/content/examples/architecture/src/app/hero-list.component.html references <app-hero-detail>.

mary-poppins · 2018-02-08T16:41:27Z

You can preview 4ffe4bd at https://pr21569-4ffe4bd.ngbuilds.io/.

jenniferfell · 2018-02-15T23:34:42Z

This will also close issue #22158

mary-poppins · 2018-03-13T19:57:32Z

You can preview 7ccf9ac at https://pr21569-7ccf9ac.ngbuilds.io/.

mary-poppins · 2018-03-14T15:25:30Z

You can preview 784d54a at https://pr21569-784d54a.ngbuilds.io/.

gkalpak · 2018-03-14T17:38:56Z

Saying that it "uses a Promise to fetch" sounds weird.
It uses the Backend service, which in turns uses something else under the hood (e.g. XMLHttpRequest, fetch, etc.) and returns a Promise to represent the future result/response.

gkalpak · 2018-03-14T17:52:36Z

This section makes it sound a little bit as if DI only works on components, while it also works (and is extensively used) in other places, such as services, pipes, NgModules.

gkalpak · 2018-03-14T17:56:32Z

they become accessible in all parts of the app

Not true for lazy-loaded modules, which get their own injector 😁
But maybe this simplification is fine for this introductory guide.

added note that you can specify providers at component level (which is now preferred, since it makes them tree-shakeable).

gkalpak · 2018-03-14T17:57:46Z

because other components --> because other modules

gkalpak · 2018-03-14T17:59:32Z

An NgModule has a root component, created during bootstrap

This mostly applies to the root NgModule only.

Does this work?
NgModules provide a compilation context for their components. A root NgModule always has a root component that is created during bootstrap, but any NgModule can include any number of additional components, which can be loaded through the router or created through the template. The components that belong to an NgModule share a compilation context.

gkalpak · 2018-03-14T18:43:27Z

Calling a "custom element" is confusing (since people might think of https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements).

gkalpak · 2018-03-14T18:44:29Z

which alters the HTML before the view is rendered

It can modify the HTML at any time, not just before the view is rendered.

gkalpak · 2018-03-14T18:46:48Z

Why not link to https://pr21569-784d54a.ngbuilds.io/guide/user-input#binding-to-user-input-events instead?

gkalpak · 2018-03-14T18:49:33Z

using the ngModel directive

This makes it sound as if two-way data binding is using (or depends on) ngModel, which is not true. Two-way data binding has nothing to do with ngModel, except for the fact that two-way data binding is often used to bind to ngModel.

gkalpak · 2018-03-14T18:52:06Z

Might also be worth mentioning (or showing in the example code-snippet above) that pipes can also take arguments.

Can you provide an example?

https://angular.io/api/common/DatePipe#examples

(I.e. {{ interpolated_value | pipe_name : arg1 : arg2 }})

mary-poppins · 2018-03-14T20:58:30Z

You can preview 96e6f45 at https://pr21569-96e6f45.ngbuilds.io/.

mary-poppins · 2018-03-15T15:27:00Z

You can preview ac2f558 at https://pr21569-ac2f558.ngbuilds.io/.

mary-poppins · 2018-03-15T16:20:26Z

You can preview 503647f at https://pr21569-503647f.ngbuilds.io/.

gkalpak

A couple of minor comments.
Also the commits should be squashed together.

LGTM otherwise.

mary-poppins · 2018-03-15T18:12:15Z

You can preview 591b9a7 at https://pr21569-591b9a7.ngbuilds.io/.

jenniferfell · 2018-03-15T20:12:41Z

Okay..finally passing all checks. Don't touch anything. :-) Merge merge quick. :-)

PR Close #21569

angular-automatic-lock-bot · 2019-09-13T10:50:57Z

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 Jan 16, 2018

gkalpak added the comp: docs label Jan 16, 2018

googlebot added cla: no and removed cla: yes labels Jan 16, 2018

StephenFluin approved these changes Jan 25, 2018

View reviewed changes

jenniferfell self-requested a review January 25, 2018 22:48

jenniferfell assigned jbogarthyde Jan 25, 2018

gkalpak added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: patch This PR is targeted for the next patch release labels Jan 26, 2018

jbogarthyde force-pushed the docs_architecture branch from d720ba1 to fdc0d52 Compare January 26, 2018 17:12

gkalpak added cla: yes and removed cla: no labels Jan 26, 2018

jenniferfell mentioned this pull request Feb 2, 2018

docs(architecture): Change hero-detail reference to app-hero-detail #21957

Closed

2 tasks

jbogarthyde force-pushed the docs_architecture branch from fdc0d52 to 4e9e77f Compare February 6, 2018 16:11

jenniferfell mentioned this pull request Feb 15, 2018

Documentation error which says NgModule instead of BrowserModule #22158

Closed

jbogarthyde force-pushed the docs_architecture branch from 5d7965b to 7ccf9ac Compare March 13, 2018 19:47

jbogarthyde force-pushed the docs_architecture branch from 7ccf9ac to 784d54a Compare March 14, 2018 15:17

jenniferfell added aio: preview and removed aio: preview labels Mar 14, 2018

jenniferfell requested a review from StephenFluin March 14, 2018 17:26

StephenFluin approved these changes Mar 14, 2018

View reviewed changes

gkalpak suggested changes Mar 14, 2018

View reviewed changes

docs(aio): update architecture section

371102b

jbogarthyde force-pushed the docs_architecture branch from 96e6f45 to ac2f558 Compare March 15, 2018 15:17

gkalpak approved these changes Mar 15, 2018

View reviewed changes

docs: incorporate suggestions and corrections from gkalpak

591b9a7

jbogarthyde force-pushed the docs_architecture branch from 503647f to 591b9a7 Compare March 15, 2018 17:47

jenniferfell added the action: merge The PR is ready for merge by the caretaker label Mar 15, 2018

mhevery pushed a commit that referenced this pull request Mar 15, 2018

docs(aio): update architecture section (#21569)

e1b82a0

PR Close #21569

mhevery pushed a commit that referenced this pull request Mar 15, 2018

docs: incorporate suggestions and corrections from gkalpak (#21569)

2731eca

PR Close #21569

mhevery closed this in 04ca77e Mar 15, 2018

mhevery pushed a commit that referenced this pull request Mar 15, 2018

docs: incorporate suggestions and corrections from gkalpak (#21569)

02e6ac2

PR Close #21569

jenniferfell mentioned this pull request Apr 25, 2018

Architecture guide references 'app-hero-detail' tag as 'hero-detail' #21955

Closed

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

Conversation

jbogarthyde commented Jan 16, 2018

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!

jbogarthyde commented Jan 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Jan 16, 2018

Uh oh!

mary-poppins commented Jan 17, 2018

Uh oh!

mary-poppins commented Jan 17, 2018

Uh oh!

gkalpak commented Jan 26, 2018

Uh oh!

mary-poppins commented Jan 26, 2018

Uh oh!

jenniferfell commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jenniferfell commented Feb 2, 2018

Uh oh!

ngbot Bot commented Feb 5, 2018

Uh oh!

ngbot Bot commented Feb 5, 2018

Uh oh!

mary-poppins commented Feb 6, 2018

Uh oh!

mary-poppins commented Feb 6, 2018

Uh oh!

wimgoeman commented Feb 8, 2018

Uh oh!

mary-poppins commented Feb 8, 2018

Uh oh!

jenniferfell commented Feb 15, 2018

Uh oh!

mary-poppins commented Mar 13, 2018

Uh oh!

mary-poppins commented Mar 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkalpak Mar 14, 2018 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jbogarthyde commented Jan 16, 2018 •

edited

Loading

jenniferfell commented Feb 1, 2018 •

edited

Loading

gkalpak Mar 14, 2018 •

edited

Loading

gkalpak Mar 14, 2018 •

edited

Loading