fix(#1042,#1052): handle value returned by navigateToAuthPage #1057
Conversation
commit: |
BracketJohn left a comment
BracketJohn left a comment There was a problem hiding this comment.
Very interesting indeed! Do you know when/how this was introduced? Because for a long time this was not an error, right?
Very good find + investigation, props! ❤️
| status: number | ||
| ok: boolean | ||
| url: any | ||
| navigationResult: boolean | string | void | undefined |
There was a problem hiding this comment.
a. Any chance we can make this type more speaking? eg making this a string-union with speaking members for different known results?
b. can/should we document this somewhere? either inside the docs or as a docstring in-code that explains the different variants and what this key means?
There was a problem hiding this comment.
a. Not really, as this is a value returned by either navigateToAuthPageWN or vue-router. I would prefer to modify this value as little as possible, since we need to immediately pass it back to vue-router.
b. I can add a doctext explaining what this value is
|
| // router.afterEach(final => final.fullPath === location ? redirect(false) : undefined) | ||
| // return href |
There was a problem hiding this comment.
is this code leftover/dead? it doesnt seem to belong to the comment above?
There was a problem hiding this comment.
No, this code was commented exactly due to the explanation above
There was a problem hiding this comment.
I didnt get that, a colon would be helpful! But alas: definitely a nit, wont block the merge for this (:
There was a problem hiding this comment.
FYI: The problem is also present in Nuxt's navigateTo, I am publishing a reproduction
Their implementation gets the same problem:
Redirect was called { fullPath: '/redirect?callback=%2Fother', 'final.fullPath': '/redirect?callback=/other', 'final.fullPath === fullPath': false } There was a problem hiding this comment.
Reproduction repo: https://github.com/phoenix-ru/nuxt-navigate-to-fullpath-reproduction
Issue will be created shortly
There was a problem hiding this comment.
please link it here as well!
🔗 Linked issue
Closes #1042, closes #1052
❓ Type of change
📚 Description
This fixes the global middleware recursion when
signInis called and its result is passed tovue-routerdirectly.Important context:
navigateToimplementation in Nuxt: https://github.com/nuxt/nuxt/blob/dc69e26c5b9adebab3bf4e39417288718b8ddf07/packages/nuxt/src/app/composables/router.ts#L130-L247beforeEachhook: https://github.com/nuxt/nuxt/blob/dc69e26c5b9adebab3bf4e39417288718b8ddf07/packages/nuxt/src/pages/runtime/plugins/router.ts#L182beforeEachdocumentation: https://router.vuejs.org/guide/advanced/navigation-guards.html#Global-Before-Guardsvue-routerrouter.resolvetest for percent-encoded characters: https://github.com/vuejs/router/blob/1885046dd5081a80c26a58ef94381c1b0e9ace2c/packages/router/src/experimental/route-resolver/resolver-fixed.spec.ts#L897-L913📝 Checklist