Skip to content

fix(router): fix lazy loading of aux routes#23459

Closed
jasonaden wants to merge 1 commit intoangular:masterfrom
jasonaden:fix_10981
Closed

fix(router): fix lazy loading of aux routes#23459
jasonaden wants to merge 1 commit intoangular:masterfrom
jasonaden:fix_10981

Conversation

@jasonaden
Copy link
Copy Markdown
Contributor

@jasonaden jasonaden commented Apr 19, 2018

Fixes #10981

This PR adds an EmptyOutletComponent that becomes the default in the case of a route config with a path, outlet, and children defined, but no Component. In the past, this would throw an error. The workaround was to define an empty component.

This PR makes that workaround the default for this scenario.

@jasonaden jasonaden requested review from IgorMinar and vicb April 19, 2018 22:56
@jasonaden jasonaden force-pushed the fix_10981 branch 2 times, most recently from 6648737 to c474619 Compare April 20, 2018 00:16
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Apr 20, 2018

You can preview 6648737 at https://pr23459-6648737.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Apr 20, 2018

You can preview c474619 at https://pr23459-c474619.ngbuilds.io/.

@IgorMinar IgorMinar added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Apr 26, 2018
@IgorMinar
Copy link
Copy Markdown
Contributor

IgorMinar commented Apr 26, 2018

http://test/OCL:194336866:BASE:194336877:1524710564950:5f5e6eaf

@IgorMinar
Copy link
Copy Markdown
Contributor

IgorMinar commented Apr 26, 2018

@jasonaden this change fails in g3, can you please investigate and get the presubmit to pass?

@IgorMinar IgorMinar added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Apr 26, 2018
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Jun 8, 2018

You can preview d10eb7b at https://pr23459-d10eb7b.ngbuilds.io/.

@JoostK
Copy link
Copy Markdown
Member

JoostK commented Jun 8, 2018

The new component is marked as entry component in the testing module, not the production variant. Is that correct? I would currently expect the dynamically referenced EmptyOutletComponent component not to be part of the AOT compilation scope, since it isn't statically referenced.

@jasonaden jasonaden added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jun 8, 2018
@jasonaden
Copy link
Copy Markdown
Contributor Author

jasonaden commented Jun 8, 2018

@JoostK Nope, it was wrong. It's been adjusted. 👍

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Jun 8, 2018

You can preview 1d27d08 at https://pr23459-1d27d08.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Jun 13, 2018

You can preview 7784ce8 at https://pr23459-7784ce8.ngbuilds.io/.

mhevery pushed a commit that referenced this pull request Jun 13, 2018
@jasonaden @mhevery
fix(router): fix lazy loading of aux routes (#23459)
d20877b 
Fixes #10981

PR Close #23459
@mhevery mhevery closed this in 5731d07 Jun 13, 2018
@jasonaden jasonaden mentioned this pull request Jun 13, 2018
Closed
@mhartington mhartington mentioned this pull request Jun 25, 2018
Closed
@csharma2212
Copy link
Copy Markdown

csharma2212 commented Jul 3, 2018

Hi @jasonaden
I want to include this fix into my app. I am using @angular/router 5.2.11.
Could you please tell in which version of angular/router this change is merged.

@JoostK
Copy link
Copy Markdown
Member

JoostK commented Jul 3, 2018

@csharma2212 Angular 6.0.5

@NickIliev NickIliev mentioned this pull request Jul 9, 2018
Open
@DrMabuse23
Copy link
Copy Markdown

DrMabuse23 commented Jul 31, 2018

Sorry but any Example is removed from web anyone have a example ?

@trotyl
Copy link
Copy Markdown
Contributor

trotyl commented Aug 16, 2018

@DrMabuse23 This is a fix, not a feature, thus nothing changed in usage, what did you expect in an example?

@daem0ndev daem0ndev mentioned this pull request Dec 16, 2018
Open
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 13, 2019

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 Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@vicb vicb Awaiting requested review from vicb

1 more reviewer

@IgorMinar IgorMinar IgorMinar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: router cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Lazy load auxilary

9 participants

@jasonaden @mary-poppins @IgorMinar @JoostK @csharma2212 @DrMabuse23 @trotyl @vicb @googlebot