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

fix: merge query for no-hash routes #286

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cadgerfeast
Copy link

Hello there :)

I wanted to propose a fix for a particular use-case with client routes.

What I found is that when using hash router, everything works fine when specifying query in route attribute, though if the client has standard routing, then query params are encoded in pathname, breaking the iframe url.

The proposal here would be to merge query params both from host, and specified route, and then assign pathname and search with the computed values.

I also added two tests but would be happy to discuss about it if you feel that should not be a supported use-case.

@MattCheely
Copy link
Collaborator

@cadgerfeast Can you clarify the use case for this and maybe provide some examples of current behavior vs expected behavior in specific scenarios? It would also be good to know if we have real-world use cases this is impacting. We can work through it here, or you're welcome to put something on my calendar or reach out in chat since we're both at Genesys.

@cadgerfeast
Copy link
Author

Hey @MattCheely, sure thing I can describe here to keep the context in PR, but I'm available to chat anytime!

There's a use case where a client has a defined URL like so: https://my-client-app.com/#/, then the host app is going to navigate to https://my-host-app.com/#/search?query=john, which leads to having the assignedRoute property to /search?query=john, and we properly set client route to https://my-client-app.com/#/search?query=john, making everything work as expected.

The issue now is with Non Hash based routers, we take the same example, and navigate to host app to https://my-host-app.com/#/search?query=john (assignedRoute property to /search?query=john), but this time the client is configured with URL https://my-client-app.com/ (standard router), with current implementation, the client will be routed to https://my-client-app.com/search%3Fquery=john, breaking the client route.

I hope that make sense and I properly described the issue here.
To give more context here, I think we don't have any issue for the moment because most app are using hash routers, but I don't think it's guaranteed it's going to always be the case.
I do have a host and client apps that do use standard routing (that's why I found the issue).

I thought I propose a solution that would fix it for later use.
The proposal here should fix it and I decided to merge both host query and configured route query param in case it's configured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants