-
Notifications
You must be signed in to change notification settings - Fork 167
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: check for client route conflicts #20188
base: main
Are you sure you want to change the base?
Conversation
Please feel free to test/review. Does need tests still. |
…eck-client-route-conflicts
flow-server/src/main/java/com/vaadin/flow/router/internal/RouteUtil.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Client file-route like Frontend/views/hello-world/{{optionalId}}.tsx
would end up matching with both hello-world
and hello-world/:optionalId?
. This might be OK with optional route param.
But it's same thing with required param with Frontend/views/hello-world/{requiredId}.tsx
+ hello-world/:requiredId?
which seems wrong.
To avoid throwing exception in this case, there can be a check for viewInfo.children()
to skip validation of client routes that have children.
So to be clear, we want to allow a flow route 'hello-world' and hilla route 'hello-world/required-id' at the same time? Edit: And I guess the same thing should work also if flow<->hilla routes are the other way around |
On second thought, I changed my mind. It's actually hard to think of any real use cases for allowing that. It would be confusing to have Docs is not saying anything regarding mixing or not mixing server and client routes this way |
Please hold on merging for now. Still need to figure out at which phase it is best to check for the conflicts. |
Update: Moved checking to end of |
vaadin-spring/src/test/java/com/vaadin/flow/spring/scopes/AbstractScopeTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Tomi Virtanen <tltv@vaadin.com>
Adding or changing client routes would not be detected even after page refresh. Could conflict check go to Hash or maybe even copy of the whole Map could be stored in VaadinContext in |
vaadin-dev-server/src/main/java/com/vaadin/base/devserver/AbstractDevServerRunner.java
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
Fixes #20144