Add lightweight token doc to library section#36144
Add lightweight token doc to library section#36144jbogarthyde wants to merge 4 commits intoangular:masterfrom
Conversation
|
You can preview 3e8e537 at https://pr36144-3e8e537.ngbuilds.io/. |
3e8e537 to
d75cfac
Compare
|
You can preview d75cfac at https://pr36144-d75cfac.ngbuilds.io/. |
d75cfac to
c8c37f6
Compare
|
You can preview c8c37f6 at https://pr36144-c8c37f6.ngbuilds.io/. |
c8c37f6 to
3c1f196
Compare
|
You can preview 3c1f196 at https://pr36144-3c1f196.ngbuilds.io/. |
3c1f196 to
38a27e5
Compare
|
You can preview 38a27e5 at https://pr36144-38a27e5.ngbuilds.io/. |
38a27e5 to
6995f77
Compare
|
You can preview 6995f77 at https://pr36144-6995f77.ngbuilds.io/. |
00367e8 to
849b773
Compare
|
You can preview 849b773 at https://pr36144-849b773.ngbuilds.io/. |
849b773 to
c0badec
Compare
|
You can preview c0badec at https://pr36144-c0badec.ngbuilds.io/. |
|
Is there anything else holding this PR from being ready for merge? @devversion and @gkalpak? |
|
@aikidave Not from my side. I was just commenting here as the proposed pattern was not supported in Angular initially. I've sent a PR that makes it work. That should be available in the next minor. #37506. We should probably mark this docs change as master-only, and it looks like it's still pending review. |
|
You can preview ad96aef at https://pr36144-ad96aef.ngbuilds.io/. |
IgorMinar
left a comment
There was a problem hiding this comment.
posting partial review feedback. mostly just nitpicks + terminology clarifications.
adds new DI technique recommendation for libraries to ensure tree-shaking for unused services includes reasons for packaging schematics with libraries, clarify schematic usage recommendation
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-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 by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
I rebased this PR and force-pushed into the PR branch. @jbogarthyde please pull change from github if you are going to make further edits to this PR. |
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-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 by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
@IgorMinar Thanks -- I am not going to make any more changes, unless you have any more comments. |
|
You can preview a0abbe7 at https://pr36144-a0abbe7.ngbuilds.io/. |
IgorMinar
left a comment
There was a problem hiding this comment.
This change LGTM now.
I'd like us to follow up on it after v10 is out and address several key points:
- the intro into the guide could be more focused "This guide documents a recommended pattern for Angular library authors that ensure that their libraries are optimizable. Angular libraries are often composed of core functionality, as well as optional features. This lightweight injection pattern ensure that the code associated with the optional features unused by a particular application, is optimized away during production builds of this application."
- some of the headings could be made more clear, I'll defer to @aikidave's guidance on this
- somewhere early on, we should state that the pattern is applicable to two use-cases: optional features expressed as directives or components that the core part of the librarie queries for via
@ContentChild,@ViewChild, etc. and optional dependencies injected into the constructor of a service or a component and flagged with the@Optional()decorator - this guide seems to mix these two use-cases, which results in confusion when the reader reads it. Again - the pattern and the recommendation to use abstract classes are applicable to both scenarios, but we should document them as two distinct use-case and show them off on two different examples.
The rest looks good. Thanks!
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-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 by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
You can preview d68bc1c at https://pr36144-d68bc1c.ngbuilds.io/. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Misko wrote up a design doc for the new lightweight-token design pattern that is recommended for library developers.
Issue Number: N/A
What is the new behavior?
The design doc content is transformed to user doc aimed at library developers and added to AIO as a new page in the Libraries section, with a link from
https://angular.io/guide/creating-libraries#refactoring-parts-of-an-app-into-a-library
Does this PR introduce a breaking change?