Sitelet https://github.com/angular/angular/pull/25609
Skip to content

fix(router): make relativeLinkResolution corrected by default - #25609

Closed
dirkluijk wants to merge 1 commit into
angular:masterfrom
dirkluijk:router-relative-link-resolution
Closed

fix(router): make relativeLinkResolution corrected by default#25609
dirkluijk wants to merge 1 commit into
angular:masterfrom
dirkluijk:router-relative-link-resolution

Conversation

@dirkluijk

@dirkluijk dirkluijk commented Aug 22, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[X] Bugfix

What is the current behavior?

To apply the fix of #22394 in Angular 6.x, you have to set relativeLinkResolution to corrected.

What is the new behavior?

This PR makes this the default for Angular 7.

Does this PR introduce a breaking change?

[X] Yes
[ ] No

Routing behaviour will be affected for empty path routes.

Other information

See https://github.com/angular/angular/pull/22394/files.

Credits to @adriensamson.

@dirkluijk dirkluijk mentioned this pull request Aug 22, 2018
Closed
2 of 3 tasks
@dirkluijk
dirkluijk force-pushed the router-relative-link-resolution branch from 219e23e to 6056d4d Compare August 22, 2018 08:50
@alfaproject

alfaproject commented Aug 22, 2018

Copy link
Copy Markdown
Contributor

You need to deprecate first and then remove in v8

@dirkluijk
dirkluijk force-pushed the router-relative-link-resolution branch from 6056d4d to 3855471 Compare August 22, 2018 12:41
@dirkluijk

dirkluijk commented Aug 22, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@alfaproject Good point. Done.

Ready for approval now.

@dirkluijk
dirkluijk force-pushed the router-relative-link-resolution branch from 3855471 to 6a99f81 Compare August 22, 2018 16:39
@dirkluijk

dirkluijk commented Nov 14, 2018

Copy link
Copy Markdown
Contributor Author

@jasonaden This PR did miss the 7.0 release. Can we make corrected default in Angular 8?

By the way, the integrations tests in this PR are now included in a752971.

@sarunint

sarunint commented Nov 14, 2018

Copy link
Copy Markdown
Contributor

What's the plan for this? In my opinion:

  1. Change this to corrected by default in v8.
  2. Deprecate this in v8, and remove it in v10?

@dirkluijk

dirkluijk commented Nov 14, 2018

Copy link
Copy Markdown
Contributor Author

Yes please. 👍

@ngbot ngbot Bot added this to the needsTriage milestone Jan 24, 2019
@atscott atscott mentioned this pull request Jul 19, 2020
Closed
3 of 14 tasks
@atscott atscott linked an issue Jul 19, 2020 that may be closed by this pull request
relativeLinkResolution: 'corrected' should be the default option #37355
Closed
@atscott atscott modified the milestones: needsTriage, v11-candidates Jul 19, 2020
@pullapprove
pullapprove Bot requested a review from atscott July 19, 2020 15:45
@atscott atscott mentioned this pull request Sep 3, 2020
Closed
@mary-poppins

mary-poppins commented Sep 10, 2020

Copy link
Copy Markdown

You can preview 219e23e at https://pr25609-219e23e.ngbuilds.io/.
You can preview 6056d4d at https://pr25609-6056d4d.ngbuilds.io/.
You can preview 3855471 at https://pr25609-3855471.ngbuilds.io/.
You can preview 6a99f81 at https://pr25609-6a99f81.ngbuilds.io/.

atscott
atscott requested changes Sep 10, 2020
View reviewed changes

@atscott atscott left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We're aiming to merge this change for v11 along with the migration
here: #38698.

Could you also update your commit message in the change to add a little more detail and also have a BREAKING CHANGE footer note to describe how this might break applications that rely on the default being 'legacy'?

Comment thread packages/router/src/router.ts Outdated
Comment thread packages/router/src/router_module.ts Outdated
@atscott
atscott force-pushed the router-relative-link-resolution branch from 8113d83 to e67529b Compare September 22, 2020 15:31
@atscott

atscott commented Sep 22, 2020

Copy link
Copy Markdown
Contributor

Note: rebased PR and removed the note about deprecations, which can be a different PR. We should decide on a plan for how it will get removed rather than just deprecating it with no plan.

@dirkluijk @atscott
fix(router): make relativeLinkResolution corrected by default
e1d7e88 
We are changing the default value from 'legacy' to 'corrected' so that new
applications are automatically opted-in to the corrected behavior from angular#22394.

BREAKING CHANGE: This commit changes the default value of
`relativeLinkResolution` from `'legacy'` to `'default'`. If your
application previously used the default by not specifying a value in the
`ExtraOptions` and uses relative links when navigating from children of
empty path routes, you will need to update your `RouterModule` to
specifically specify `'legacy'` for `relativeLinkResolution`.
See https://angular.io/api/router/ExtraOptions#relativeLinkResolution
for more details.
@atscott
atscott force-pushed the router-relative-link-resolution branch from e67529b to e1d7e88 Compare September 22, 2020 15:38
@mary-poppins

mary-poppins commented Sep 22, 2020

Copy link
Copy Markdown

You can preview e1d7e88 at https://pr25609-e1d7e88.ngbuilds.io/.

@atscott

atscott commented Sep 23, 2020

Copy link
Copy Markdown
Contributor

presubmit

@atscott atscott added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Sep 24, 2020
@alxhub alxhub closed this in 837889f Sep 24, 2020
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Oct 25, 2020

Copy link
Copy Markdown

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

Reviewers

@atscott atscott atscott approved these changes

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: router breaking changes cla: yes cross-cutting: types freq4: critical target: major This PR is targeted for the next major release type: bug/fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

relativeLinkResolution: 'corrected' should be the default option

7 participants

@dirkluijk @alfaproject @sarunint @mary-poppins @atscott @kara @googlebot