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

Expand doc for APP_INITIALIZER token #34760

Closed
wants to merge 2 commits into from

Conversation

jbogarthyde
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

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
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

The description of https://angular.io/api/core/APP_INITIALIZER is brief and uninformative.

Issue Number: #34703

What is the new behavior?

Expands the API doc description.
Needs an example or pointer to additional doc to be really useful.
The subject does not appear to be covered anywhere in the current doc, but lots of external discussion.

Does this PR introduce a breaking change?

  • Yes
  • No

@jbogarthyde jbogarthyde requested a review from a team as a code owner January 13, 2020 21:26
@jbogarthyde jbogarthyde self-assigned this Jan 13, 2020
@ngbot ngbot bot modified the milestone: Backlog Jan 13, 2020
@jbogarthyde jbogarthyde added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jan 13, 2020
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

👍 LGTM, thanks for the update.

I think it'd be great to add an example in the followup PR on how this token can be used.

@mary-poppins
Copy link

You can preview e738e32 at https://pr34760-e738e32.ngbuilds.io/.

@jbogarthyde jbogarthyde added this to Waiting for Approval in docs Jan 13, 2020
@jbogarthyde jbogarthyde added action: merge The PR is ready for merge by the caretaker and removed state: needs eng input state: needs eng review Needs technical review and approval from engineering team action: merge The PR is ready for merge by the caretaker labels Jan 13, 2020
@@ -12,7 +12,13 @@ import {Inject, Injectable, InjectionToken, Optional} from './di';


/**
* A function that will be executed when an application is initialized.
* An injection token that allows you to inject one or more functions to be executed during
Copy link
Member

Choose a reason for hiding this comment

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

I think we should say provide one or more functions to be executed. Those functions then get injected at application startup and run. This verbiage is similar to how the value is actually used in code:

{
   provide: APP_INITIALIZER,
   useValue: () => {}
}

* app initialization. If any of these functions returns a Promise, initialization
* does not complete until the Promise is resolved.
*
* You can, for example, create a factory function that loads an external configuration
Copy link
Member

Choose a reason for hiding this comment

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

This whole example has me a little confused, mostly I think just by the wording. If I get your intention correctly, maybe this would be more direct?

You can, for example, create a factory function that loads language data or an external configuration, and provide that function to the APP_INITIALIZER token. 
That way, the function is executed during the application bootstrap process, and the needed data is available on startup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

@mary-poppins
Copy link

You can preview 75831e2 at https://pr34760-75831e2.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 5a7de1a at https://pr34760-5a7de1a.ngbuilds.io/.

@jbogarthyde jbogarthyde added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 14, 2020
@jbogarthyde jbogarthyde moved this from Waiting for Approval to Waiting for Merge in docs Jan 14, 2020
atscott pushed a commit that referenced this pull request Jan 14, 2020
@atscott atscott closed this in 23cbfa7 Jan 14, 2020
@jbogarthyde jbogarthyde deleted the jb-apidoc-init branch January 15, 2020 17:07
@jbogarthyde jbogarthyde moved this from Waiting for Merge to Done in docs Jan 20, 2020
@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 Feb 15, 2020
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 aio: preview cla: yes effort2: days freq2: medium risk: low target: patch This PR is targeted for the next patch release type: bug/fix
Projects
docs
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants