-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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: option to disable SPA catch-all #8061
Conversation
I would like to see something like this as well. I had filed an issue about this in #7631 hoping we could fix it in 3.0. I tried it awhile back in #4640 as well, but it didn't get much momentum so we're just removing the middlewares instead, which is more brittle. I think |
The post hooks, and therefore the SSR middleware, are installed before these two middlewares, so I don't see how these two would conflict with the SSR middleware:
I lean towards keeping them since they are needed for MPAs. The pro about I'm thinking a more robust design would be a more high-level config: We can start with Thoughts? Also, one thing I forgot to mention: I think we shouldn't add these options to the Vite CLI. Because it should be the SSR plugin/framework that sets the config, not the user. Saving us from creating a new Vite CLI option. |
(The failing tests are flaky tests. Things are green on my fork.) |
c916db9
to
aee24d9
Compare
I'm just remembering patak's idea of a global |
I just tested and SvelteKit does work if we leave these two middlewares in the general case. However, I do wonder if there is still some case where you'd want it disabled. If you add the user's application as a middleware to an Express server, they may want to add middleware handlers for other URLs and 404 and may want it disabled in this case. So it seems safer to me to disable it for the SSR case. If we go with the |
@benmccann Makes sense 👍. |
Closing in favor of #8217. @benmccann I've left the 404 middleware when |
Description
Resurrection of #7665: it was merged but later reverted because SPAs should be able to do client-side routing and catch-all routing is needed for that.
Such catch-all routing doesn't make sense for MPAs or SSR apps. Hence this new option to disable it.
@benmccann This removes the SPA fallback middleware, so this may be a solution for your problem.
The name
disableSPACatchAll
is fairly long for Vite's standards. But I think it's worth it. I think it's pretty clear and considerably increases the probability of understanding whatdisableSPACatchAll
is about without having to look up documentation.Additional context
This will be used by Vike Frameworks (frameworks based on Vike tools such as
vite-plugin-ssr
0.4).What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).I considered adding a test, but I realized that it would require a whole new playground setup. I'm thinking that adding a playground just for this would add too much bloat to be worth it. Note that vite-plugin-ssr will have tests for this, so we'll catch any problems at latest when running vite-plugin-ssr's test suite.