-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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!: appType (spa, mpa, custom), boolean middlewareMode #8452
Conversation
packages/vite/src/node/config.ts
Outdated
@@ -536,7 +544,10 @@ export async function resolveConfig( | |||
} | |||
}, | |||
worker: resolvedWorkerOptions, | |||
spa: config.spa ?? true | |||
appType: | |||
config.appType ?? config?.server?.middlewareMode === 'ssr' |
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.
middlewareMode
is a boolean now
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.
This is for backward compat. We can break it directly for v3 thought
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.
I think they are only a handful of frameworks out there using middlewareMode: 'ssr'
now that it should be safe to break. Otherwise another option is to log
a warning, but framework authors would likely fix it right away so that end-users don't see it.
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
How about we make only While we keep So that SvelteKit (and others) can still use (Btw. I really like replacing CC @aleclarson. |
I actually like that |
Actually, how about a Before: if (process.env.NODE_ENV === 'production') {
app.use(sirv(`./dist/client/`))
} else {
const vite = await import('vite')
viteDevServer = await vite.createServer({
server: { middlewareMode: true },
})
app.use(viteDevServer.middlewares)
} After: if (process.env.NODE_ENV === 'production') {
app.use(sirv(`./dist/client/`))
} else {
app.use((await import('vite')).createDevMiddleware())
} It's low priority and I'm personally fine with the current API. But I can see many to welcome such aesthetic improvement and Vite 3 is a nice opportunity for that. |
Co-authored-by: Shinigami <[email protected]>
Co-authored-by: Shinigami <[email protected]>
Co-authored-by: Shinigami <[email protected]>
That sounds confusing to me. I think it's better if it just always have the same value. I don't mind overriding the default value if needed. The question to me though is what is the better default and I think that generally
It's not a compelling need, but it might be slightly nicer. I wouldn't be opposed to it as a follow-up |
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.
lgtm. thank you so much for this!
👍 Ok that makes sense. You're right, it's clearer to separate both. Sorry for the noise. As for Anyways, LGTM; green-check-marking this. |
I agree with you @brillout, it feels strange to use a |
👍 Although I have a slight preference for |
Co-authored-by: Bjorn Lu <[email protected]>
Co-authored-by: Bjorn Lu <[email protected]>
Co-authored-by: Bjorn Lu <[email protected]>
Co-authored-by: Bjorn Lu <[email protected]>
Description
Close #8438
Context:
preview
anddev
for MPA and SSR apps #8217This PR implements:
appType: 'spa' | 'mpa' | 'custom'
server.middlewareMode: 'ssr'
appType: 'custom'
+server.middlewareMode: true
server.middlewareMode: 'html'
appType: 'spa' | 'mpa'
+server.middlewareMode: true
server.middlewareMode
was originally aboolean
and we extended it to be'ssr' | 'html'
once people wanted control over the included middlewares.#8217 introduced a
spa: boolean
option, but we found out while discussing in #8438 that it isn't enough and that there is overlap between this new option and the extensions previously added tomiddlewareMode
This PR reverts
middlewareMode
to a boolean, that now only controls if there is a server managed by Vite and has no effect on what middlewares are included. A newappType
option allows us to define at a high level the different sets of middlewares (and options like sirvsingle: true
in preview).I think that
'custom'
is less confusing than'ssr'
for the option that removes all the HTML middlewares and leaves the HTML handling to the framework.What is the purpose of this pull request?