Skip to content

fix(router): restore 'history.state' (#28108)#28176

Closed
alecvolo wants to merge 3 commits intoangular:masterfrom
alecvolo:fix-28108-branch
Closed

fix(router): restore 'history.state' (#28108)#28176
alecvolo wants to merge 3 commits intoangular:masterfrom
alecvolo:fix-28108-branch

Conversation

@alecvolo
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • [x ] 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?

Issue Number: #28108

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • [x ] No

Other information

@alecvolo alecvolo requested a review from a team January 16, 2019 09:39
@ngbot ngbot bot added this to the needsTriage milestone Jan 24, 2019
@pullapprove pullapprove bot requested a review from atscott August 6, 2020 20:27
@atscott atscott removed the request for review from a team August 6, 2020 20:27
@atscott atscott added freq2: medium target: major This PR is targeted for the next major release router: navigation pipe events/scheduleNavigation/transitions observable labels Aug 6, 2020
@atscott
Copy link
Contributor

atscott commented Aug 6, 2020

This seems like the correct fix. I've rebased the PR and started kicked off the internal tests.

@atscott atscott added the action: presubmit The PR is in need of a google3 presubmit label Aug 6, 2020
@googlebot
Copy link

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 @googlebot I consent. in this pull request.

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 cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels Aug 7, 2020
…om Angular router (angular#28108)

When navigations coming from Angular router we may have a payload stored in state property. When this
 exists, set extras's state to the payload.
@atscott
Copy link
Contributor

atscott commented Aug 7, 2020

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Aug 7, 2020
@atscott atscott added risk: medium and removed action: presubmit The PR is in need of a google3 presubmit labels Aug 7, 2020
@atscott
Copy link
Contributor

atscott commented Aug 7, 2020

caretaker note: as this is a router change, please merge and sync to g3 on its own for easy rollback if it breaks things.

@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Aug 7, 2020
@AndrewKushnir AndrewKushnir added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Aug 7, 2020
@AndrewKushnir
Copy link
Contributor

@atscott it looks like a couple CircleCI jobs are failing. Could you please have a look when you get a chance? Thank you.

@atscott
Copy link
Contributor

atscott commented Aug 8, 2020

Hey @josephperrott, I'm not familiar with the integration:side-effects_test. Can you advise how to proceed here? I'm guessing it has something to do with the object destructuring done in the change.

@josephperrott
Copy link
Member

@atscott Yes, the destructuring causes tslib to have to be loaded to use router, making it a "side effect" of the usage.

If we are okay with adding this side effect to @angular/router, then //integration/side-effects/snapshots/router/esm2015.js needs to be updated to reflect tslib needs to load. If we would not like for tslib to be a side effect, then the code needs to be refactored to no longer rely on the destructuring.

@atscott atscott added action: presubmit The PR is in need of a google3 presubmit and removed action: merge The PR is ready for merge by the caretaker labels Aug 10, 2020
@atscott atscott 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 action: presubmit The PR is in need of a google3 presubmit labels Aug 10, 2020
@AndrewKushnir AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Aug 10, 2020
@atscott
Copy link
Contributor

atscott commented Aug 10, 2020

global presubmit

@atscott atscott removed the action: presubmit The PR is in need of a google3 presubmit label Aug 11, 2020
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
…om Angular router (angular#28108) (angular#28176)

When navigations coming from Angular router we may have a payload stored in state property. When this
 exists, set extras's state to the payload.

PR Close angular#28176
@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 Sep 11, 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 area: router cla: yes freq2: medium risk: medium router: navigation pipe events/scheduleNavigation/transitions observable target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

'history.state' object is getting lost when navigate using back/forward buttons

6 participants