-
Notifications
You must be signed in to change notification settings - Fork 545
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
feat: support regex for prerender.ignore
#1033
Conversation
β Live Preview ready!
|
Notice: maybe conflict with #1032 |
Will first merge #1032 to rebase your branch |
Fixed the conflicts and improve the docs at the same time. |
Codecov Report
@@ Coverage Diff @@
## main #1033 +/- ##
==========================================
- Coverage 67.59% 67.55% -0.05%
==========================================
Files 62 62
Lines 6259 6263 +4
Branches 707 707
==========================================
Hits 4231 4231
- Misses 2015 2019 +4
Partials 13 13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Wondering if we could add a test case for it as well. |
prerender.ignore
prerender.ignore
I'm thinking we might be better first allow |
Allowing the pattern |
I'm afraid not. According to linked issue, it requires custom logic which can be implemented by a custom hook / callback (internally can use also regex...) |
So what are the next steps? |
As @pi0 said, custom hooks/callbacks would be the most powerful solution; but, for most use cases, RegEx should be enough.
In fact, my use cases needs the power of "excluded ignore", which is not easily solvable with route rules matching. The underlying library, whose radix-tree-matching do not produce result with the same order as in the input. |
Should I implement more strict input checking? If the user uses TypeScript, the current solution will work fine. |
I think it's fine for types using TS. If I understand correctly, you don't want to support regex but the ability to give a function instead @pi0 ? |
A hook would be nice for advanced usecases. I prefer to keep What do you think about introducing prerender: {
include: ['/static/**'],
exclude: ['/**']
} |
But how about the priority? Is the 'include' a 'negative exclude' like Since this option won't affect how route options work (in fact, it's a option for the crawler, not the server), I think RegExs should be fine. |
The most precise rule takes into priority
The point is route rule definition consistency and having most simple way to do things when possible. |
OK. How about add a |
The reason is that, routeRules is for server, but the prerender is a standalone thing though. |
We do actually support We can indeed support |
So the problem is I want to make |
Yes that would be possible! (SSR renderer won't be affected by |
Sorry for the delay. Now the problems becomes that the https://github.com/unjs/radix3#route-matcher is not matching the most specific rule! Let's see a example below: Simple swapping the rules will affect the output. |
Nice catch @thezzisu It seems we need a way to prioritize the routes for best matching strategy, similar to what vue-router 4 is doing π€ |
I think this would be a better solution than supporting regex. I'll close this pull request, and open a issue on unjs/radix3 instead. |
Radix 3 basically does that :) but we don't use it internally in nitro prerender inference... |
So open back an issue in Nitro pre-render to support this I guess π |
~> #1078 |
π Linked issue
Resolves #308
β Type of change
π Description
Enhance prerender.ignore with the ability to use RegExp.
π Checklist