feat: use preview server parameter in preview server hook #11647
Conversation
| // apply server hooks from plugins | ||
| const postHooks: ((() => void) | void)[] = [] | ||
| for (const hook of config.getSortedPluginHooks('configurePreviewServer')) { | ||
| postHooks.push(await hook({ middlewares: app, httpServer })) | ||
| postHooks.push(await hook(server)) |
There was a problem hiding this comment.
configurePreviewServer hooks now runs after the server started to listen on the port.
I'm not sure if this is safe or not. But I think it's better to align with the dev server.
How about adding PreviewServerForHook type that has a nullable resolvedUrls property? And then use PreviewServerForHook for the hooks and use PreviewServer for the return type of preview function for compatibility.
There was a problem hiding this comment.
Ah you're right. I think PreviewServerForHook makes sense, but for the next major, maybe we can make it nullable by default so the types are cleaner.
With this change, I think this PR is good too.
There was a problem hiding this comment.
I think we could merge this in 4.3. @sapphi-red if you agree, you can directly commit your suggestion and merge this one. The alignment is quite uncontroversial to me, we shouldn't need a meeting to review this one IMO.
There was a problem hiding this comment.
I rebased on main and pushed some commits 👍
sapphi-red
added
the
p2-nice-to-have
sapphi-red
force-pushed
the
configure-preview-hook-type
branch
from
aab2019
to
f963c90
Compare
Description
Closes #11631. The code changes basically follow what I described in the issue's proposed solution.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).