Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router: link urls doesn't have query string parameters #7367

Closed
alexpods opened this issue Mar 2, 2016 · 8 comments
Closed

Router: link urls doesn't have query string parameters #7367

alexpods opened this issue Mar 2, 2016 · 8 comments

Comments

@alexpods
Copy link

alexpods commented Mar 2, 2016

Copy of @zanettin's issue from angular/router#397

When I click on a routerLink

LINK:

<a [routerLink]=" ['./RouteParamsTest', { userId : '1919', lang : 'it', token : 'jwt' }] ">Route Params Test 2</a>

CONFIG:

{ path: '/route-params-test/:userId/:lang', component: RouteParamsTest, name: 'RouteParamsTest' }

with disabled JS, this URL gets called => http://localhost:3000/route-params-test/1919/it and the query string won't appear. With activated JS everything works as expected.

The reason of the problem is here. As you can see urlParams are used only in _stringifyMatrixParams() which is called only if we have auxInstructions (see here). So if we don't have auxilary routes then urlParams will never be used. (toLinkUrl() is called here to update href of the link)

@matsko @btford @IgorMinar

@btford
Copy link
Contributor

btford commented Mar 3, 2016

@petebacondarwin was this possibly caused by your recent refactoring?

@petebacondarwin
Copy link
Member

I doubt the refactoring is the cause, since we only just merged the refactoring into master yesterday.
It might even be that the refactoring helps with this. @alexpods and @zanettin can you try with the latest Router bits in Angular 2 master?

@alexpods
Copy link
Author

alexpods commented Mar 4, 2016

@btford @petebacondarwin

Refactoring is definitely not the cause. The original @zanettin's issue was created 21 days ago. @petebacondarwin I doubt that it will work in the latest master (actually I'm sure it won't).

The cause of the error:

  1. instruction.toLinkUrl() method is used to create href attributes here
  2. look at toLinkUrl implementation: urlParams are used only as a result of this._stringifyAux() invokation.
  3. we don't have auxInstructions in this case, so urlParams won't be applied.

@petebacondarwin
Copy link
Member

OK I accept that it is a bug.

Just for my sake, can you explain a bit about why this error does not appear when you have JS enabled? Something different going on in Universal?

petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Mar 4, 2016
@alexpods
Copy link
Author

alexpods commented Mar 4, 2016

@petebacondarwin
When javascript is enabled and you click on a link navigateByInstruction() is used. So the user will be navigated to the correct page/state.

When javascript is disabled href value will be used to move user on another page (simple <a> link). Because href value doesn't have query string, result page url also won't have query string.

@petebacondarwin
Copy link
Member

I have created a fix for this at #7431. @btford can you give it a check, in case I have missed something.

@alexpods
Copy link
Author

alexpods commented Mar 4, 2016

Thanks, @petebacondarwin .

Just to demonstrate the issue I've created this plunk. Look at href attribute of the link. It doesn't have query string with param1 and param2.

@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 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants