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

There should be a simpler alternative to the(.*)? pattern #1301

Open
zcrittendon opened this issue Feb 8, 2022 · 3 comments
Open

There should be a simpler alternative to the(.*)? pattern #1301

zcrittendon opened this issue Feb 8, 2022 · 3 comments
Labels
discussion This problem still needs more feedback

Comments

@zcrittendon
Copy link

zcrittendon commented Feb 8, 2022

Version

4.0.12

Reproduction link

jsfiddle.net

Steps to reproduce

  1. Create route containing multiple (.*)* patterns. For example path: '/:seoPath(.*)*/c/:categoryId/:facetsPath(.*)*'
  2. Trigger processing of route containing the (.*)* pattern with a long path matching the pattern.
  3. Observe a delay of seconds (~ 20 path segments) or minutes (~ 30 path segments)

This delay occurs when rendering router-link as well as during route navigation.

This fiddle has ~20 path segments and shows a 1-second delay when loading the page or clicking the link:
https://jsfiddle.net/zcrittendon/j1fmu680/2/

This fiddle has ~30 path segments and shows shows a multi-minute delay (or browser hang) when loading the page:
https://jsfiddle.net/zcrittendon/dmzjvq56/8/

This fiddle has has ~30 path segments, but has no delay because it uses pattern (.*)? instead of (.*)*
https://jsfiddle.net/zcrittendon/9ze24hx7/5/

What is expected?

Router performance should not be longer than ~1 second even for very long wildcard URL paths.

What is actually happening?

Router performance is extremely long (or hands) for very long wildcard URL paths.

@posva
Copy link
Member

posva commented Feb 8, 2022

If you don't need the array format you should just use (.*)?, otherwise the generated regexp becomes too complex which is what makes it slow.

@posva posva closed this as completed Feb 8, 2022
@posva
Copy link
Member

posva commented Feb 8, 2022

Although I think we could maybe introduce a new syntax: * to just match anything without creating a complex regex group. This needs more thought.

@posva posva reopened this Feb 8, 2022
@posva posva changed the title Poor performance for long routes containing (.*)* pattern There should be a simpler alternative to the(.*)? pattern Feb 8, 2022
@posva posva added the discussion This problem still needs more feedback label Feb 8, 2022
@Mash19
Copy link

Mash19 commented Apr 11, 2022

I was a little confused by the grouping but looking at the fiddles, I'm seeing that the slashes aren't part of the regex used for the parameters. I guess they are being handled internally, so instead of using the matchAll metacharacter you can use a character class and just include the characters you using in your route. This performs pretty well even with the greedy regex pattern.
For the jsfiddle examples replacing the path with
path: '/:seoPath([a-z]*)*/c/:categoryId/:facetsPath([-\\w]*)*',
There is no hang loading or with the long route and the submatches are in the parameter array.
Of course the regex is customized to the routes in the examples, and could be a little broader [a-zA-Z] instead of [a-z] to handle upper and lower case. Or [\w] if you are matching alphanumeric like the facetsPath parameter.
There are a few other characters that are valid in an URL path that could be added but you'd probably be better off keeping it simple and only adding what you actually need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This problem still needs more feedback
Projects
None yet
Development

No branches or pull requests

3 participants