-
Notifications
You must be signed in to change notification settings - Fork 26.2k
Add lightweight token doc to library section #36144
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
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/. |
@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/. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!
a0abbe7
to
d68bc1c
Compare
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/. |
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 PR Close #36144
…37506) Currently Angular internally already handles `InjectionToken` as predicates for queries. This commit exposes this as public API as developers already relied on this functionality but currently use workarounds to satisfy the type constraints (e.g. `as any`). We intend to make this public as it's low-effort to support, and it's a significant key part for the use of light-weight tokens as described in the upcoming guide: angular#36144. In concrete, applications might use injection tokens over classes for both optional DI and queries, because otherwise such references cause classes to be always retained. This was also an issue in View Engine, but now with Ivy, this pattern became worse, as factories are directly attached to retained classes (ultimately ending up in the production bundle, while being unused). More details in the light-weight token guide and in: https://github.com/angular/angular-cli/issues/16866. Closes angular#21152. Related to angular#36144. PR Close angular#37506
…36144) 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 PR Close angular#36144
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. |
…36144) 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 PR Close angular#36144
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?