fix(router): do not finish bootstrap until all the routes are resolved by vsavkin · Pull Request #14762 · angular/angular

vsavkin · 2017-02-27T12:31:36Z

Please check if the PR fulfills these 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)

What kind of change does this PR introduce? (check one with "x")

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

The bootstrap doesn't wait for the router to finish the initial navigation.

What is the new behavior?

There is an option you can pass to RouterModule.foRoot to make bootstrap wait. Since many depend on the current behavior, it is preserved as a default and is deprecated.

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[ ] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

vicb · 2017-02-27T19:45:12Z

typos in "localtion"

could you be more verbose on what the preferred choice should be (ie most probably "enabled") and when you would like to switch to "disable". Also say that the "legacy_..." options are for backward compatibility and should not be used for new apps ?

vicb · 2017-02-27T19:45:30Z

back quote true/false

vicb · 2017-02-28T22:44:52Z

move these 2 mehtods out of the class ?

vicb · 2017-02-28T22:48:05Z

initNavigationDone = false ?

vicb · 2017-02-28T22:52:36Z

Please add a DEPRECATIONS section in the commit message. Thanks !

DEPRECATION Use `RouterModule.forRoot(routes, {initialNavigation: 'enabled'})` instead of `RouterModule.forRoot(routes, {initialNavigtaion: true})`. Before doing this, move the initialization logic affecting the router from the bootstrapped component to the boostrapped module. Similarly, use `RouterModule.forRoot(routes, {initialNavigation: 'disabled'})` instead of `RouterModule.forRoot(routes, {initialNavigation: false})`. Deprecated options: 'legacy_enabled', `true` (same as 'legacy_enabled'), 'legacy_disabled', `false` (same as 'legacy_disabled'). The "Router Initial Navigation" design document covers this change. Read more here: https://docs.google.com/document/d/1Hlw1fPaVs-PCj5KPeJRKhrQGAvFOxdvTlwAcnZosu5A/edit?usp=sharing

IgorMinar · 2017-03-02T02:12:35Z

    enableTracing?: boolean;
    errorHandler?: ErrorHandler;
-    initialNavigation?: boolean;
+    initialNavigation?: InitialNavigation;


isn't this a breaking change? when was this interface introduced?

see https://github.com/angular/angular/pull/14762/files#diff-c0baae5e1df628e1a217e8dc38557fcbR237

the changes are described in the commit comment: former options are valid and have the same meaning (true/false), new values have a new meaning and are documented,

angular#14762) Cherry-pick of 5df998d onto 2.4.x branch. DEPRECATION: Use `RouterModule.forRoot(routes, {initialNavigation: 'enabled'})` instead of `RouterModule.forRoot(routes, {initialNavigtaion: true})`. Before doing this, move the initialization logic affecting the router from the bootstrapped component to the boostrapped module. Similarly, use `RouterModule.forRoot(routes, {initialNavigation: 'disabled'})` instead of `RouterModule.forRoot(routes, {initialNavigation: false})`. Deprecated options: 'legacy_enabled', `true` (same as 'legacy_enabled'), 'legacy_disabled', `false` (same as 'legacy_disabled'). The "Router Initial Navigation" design document covers this change. Read more here: https://docs.google.com/document/d/1Hlw1fPaVs-PCj5KPeJRKhrQGAvFOxdvTlwAcnZosu5A/edit?usp=sharing

angular#14762) DEPRECATION: Use `RouterModule.forRoot(routes, {initialNavigation: 'enabled'})` instead of `RouterModule.forRoot(routes, {initialNavigtaion: true})`. Before doing this, move the initialization logic affecting the router from the bootstrapped component to the boostrapped module. Similarly, use `RouterModule.forRoot(routes, {initialNavigation: 'disabled'})` instead of `RouterModule.forRoot(routes, {initialNavigation: false})`. Deprecated options: 'legacy_enabled', `true` (same as 'legacy_enabled'), 'legacy_disabled', `false` (same as 'legacy_disabled'). The "Router Initial Navigation" design document covers this change. Read more here: https://docs.google.com/document/d/1Hlw1fPaVs-PCj5KPeJRKhrQGAvFOxdvTlwAcnZosu5A/edit?usp=sharing

angular-automatic-lock-bot · 2019-09-10T23:22:45Z

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 Feb 27, 2017

vsavkin force-pushed the 4x_router_bootstrap_fix branch 3 times, most recently from 7a1483f to b0c4fd9 Compare February 27, 2017 13:25

vicb added area: router action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 27, 2017

vicb reviewed Feb 27, 2017

View reviewed changes

Comment thread modules/@angular/router/src/router_module.ts Outdated

Copy link
Copy Markdown

Contributor

vicb Feb 27, 2017

Choose a reason for hiding this comment

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

back quote true/false

vicb mentioned this pull request Feb 27, 2017

fix(router): do not finish bootstrap until all the routes are resolve… #14625

Closed

vicb added the state: blocked label Feb 28, 2017

vicb reviewed Feb 28, 2017

View reviewed changes

Comment thread modules/@angular/router/src/router_module.ts Outdated

Copy link
Copy Markdown

Contributor

vicb Feb 28, 2017

Choose a reason for hiding this comment

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

move these 2 mehtods out of the class ?

vicb reviewed Feb 28, 2017

View reviewed changes

Comment thread modules/@angular/router/src/router_module.ts Outdated

Copy link
Copy Markdown

Contributor

vicb Feb 28, 2017

Choose a reason for hiding this comment

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

initNavigationDone = false ?

vsavkin force-pushed the 4x_router_bootstrap_fix branch from b0c4fd9 to bd367dc Compare March 1, 2017 12:39

vsavkin force-pushed the 4x_router_bootstrap_fix branch from bd367dc to dc49334 Compare March 1, 2017 17:46

vicb 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 state: blocked labels Mar 1, 2017

vicb approved these changes Mar 1, 2017

View reviewed changes

IgorMinar reviewed Mar 2, 2017

View reviewed changes

IgorMinar approved these changes Mar 7, 2017

View reviewed changes

chuckjaz merged commit 5df998d into angular:master Mar 7, 2017

vsavkin mentioned this pull request Mar 13, 2017

fix(router): do not finish bootstrap until all the routes are resolve… #15121

Merged

3 tasks

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(router): do not finish bootstrap until all the routes are resolved#14762

fix(router): do not finish bootstrap until all the routes are resolved#14762
chuckjaz merged 1 commit into
angular:masterfrom
vsavkin:4x_router_bootstrap_fix

vsavkin commented Feb 27, 2017

Uh oh!

vicb Feb 27, 2017

Uh oh!

vicb Feb 27, 2017

Uh oh!

vicb Feb 28, 2017

Uh oh!

vicb Feb 28, 2017

Uh oh!

vicb commented Feb 28, 2017

Uh oh!

IgorMinar Mar 2, 2017

Uh oh!

vicb Mar 2, 2017

Uh oh!

angular-automatic-lock-bot Bot commented Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

vsavkin commented Feb 27, 2017

Uh oh!

vicb Feb 27, 2017

Choose a reason for hiding this comment

Uh oh!

vicb Feb 27, 2017

Choose a reason for hiding this comment

Uh oh!

vicb Feb 28, 2017

Choose a reason for hiding this comment

Uh oh!

vicb Feb 28, 2017

Choose a reason for hiding this comment

Uh oh!

vicb commented Feb 28, 2017

Uh oh!

IgorMinar Mar 2, 2017

Choose a reason for hiding this comment

Uh oh!

vicb Mar 2, 2017

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot Bot commented Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants