fix: resolvedUrls is null in plugin's configureServer after server restart

[XiSenao]

Description

fixes #15438
refs #14890

Additional context

#14890 PR fixed the issue of indirectly referencing the dev server instance (e.g. referencing it In the delayed execution function), which was not effective when directly referencing the dev server instance after restarting the server (e.g. partial middlewares and configureServer hook in the plugin). vite will expose the dev server instance to the configureServer hook in the plugin, which also means that the dev server instance may be used by other hooks in the plugin. Therefore, I think it is necessary to maintain consistency of the dev server instance after server restart. as for the middlewares, I don't think any additional processing is necessary at the moment, as it may result in performance loss and inconsistent behavior before and after configuration. This fix adds a layer of proxy to the dev server reference in the configureServer hook of the plugin.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines, especially the Pull Request Guidelines.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Update the corresponding documentation if needed.
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[XiSenao]

After restarting the server, does Vite not need to expose the new dev server instance to user plugins? @bluwy @sapphi-red

[bluwy]

Thanks for investigating this! The indirections in server handling today took me quite some time to understand 😄 So from what I understand, the configureServer hook is referencing the old server on init, so after _setInternalServer, it's still not referencing the new server, causing the problem.

The proxy seems like the only way best way to handle this. Though at this point, maybe it's easier to have createServer() simply return a Proxy, which proxies to whichever latest server internally. And that should simplify a lot of our old/new server handling. I think I'll let @patak-dev chime in on this before deciding the best way forward.

After restarting the server, does Vite not need to expose the new dev server instance to user plugins?

It should expose the new one. It's a bug that it doesn't since #14890

[patak-dev]

Thanks for the PR! Ya... this is getting trickier and trickier. I vote to merge this PR for the next minor and test it during the beta period we are just starting. And then evaluate @bluwy's idea of making createServer return a proxy for the next major. It would also be easier to explain and document how the server magical instance behaves.

[XiSenao]

The indirections in server handling today took me quite some time to understand 😄

oh, I am very sorry that my description has caused you trouble in understanding the meaning. In order to avoid further misunderstandings, I would like to provide a more detailed explanation of this issue.

During the server restart phase, a new dev Server will be created through the _createServer function.

vite/packages/vite/src/node/server/index.ts

Lines 988 to 992 in a621de8

	let newServer = null
	try {
	// delay ws server listen
	newServer = await _createServer(inlineConfig, { ws: false })
	} catch (err: any) {

The configureServer hook will be executed within the _createServer function

vite/packages/vite/src/node/server/index.ts

Lines 716 to 718 in a621de8

	for (const hook of config.getSortedPluginHooks('configureServer')) {
	postHooks.push(await hook(server))
	}

and at this point, the dev server instance passed to the configureServer hook can be considered as an new initialized dev server instance.

vite/packages/vite/src/node/server/index.ts

Line 433 in a621de8

let server: ViteDevServer = {

After creating a new dev server, existing dev server instances will be reused using the Object.assign method.

vite/packages/vite/src/node/server/index.ts

Lines 1002 to 1006 in a621de8

	// Assign new server props to existing server instance
	const middlewares = server.middlewares
	newServer._configServerPort = server._configServerPort
	newServer._currentServerPort = server._currentServerPort
	Object.assign(server, newServer)

Due to reusing the existing dev server instance, the subsequent updates to the properties are on the existing dev server instance rather than a new dev server instance.
In the #14890 PR, the _setInternalServer method is used to rebind the dev server instance inside the _createServer function, which is a good idea.

vite/packages/vite/src/node/server/index.ts

Lines 611 to 615 in a621de8

	_setInternalServer(_server: ViteDevServer) {
	// Rebind internal the server variable so functions reference the user
	// server instance after a restart
	server = _server
	},

However, the configureServer hook cannot access the re-bound dev Server instance in the _setInternalServer function due to closure reasons, leading to the problem in #15438.

vite/packages/vite/src/node/server/index.ts

Line 717 in a621de8

postHooks.push(await hook(server))

This is also what I mentioned in the description "which was not effective when directly referencing the dev server instance after restarting the server". Directly referencing the dev Server instance cannot access the server in the _setInternalServer function due to closure reasons, and that is the current problem.

The indirect reference to the dev server instance mentioned in the description refers to a situation similar to the following.

vite/packages/vite/src/node/server/index.ts

Lines 450 to 452 in a621de8

	transformRequest(url, options) {
	return transformRequest(url, server, options)
	},

When referencing the dev server instance in the function, the function will be triggered at some point in the future. At this point, the dev server instance instance obtained is the one re-bound in the _setInternalServer method, which is the correct behavior.

Though at this point, maybe it's easier to have createServer() simply return a Proxy, which proxies to whichever latest server internally.

It may be a lack of thinking on my part, I don't know why making createServer return a proxy can change the dev server reference in the configureServer hook.

[bluwy]

oh, I am very sorry that my description has caused you trouble in understanding the meaning. In order to avoid further misunderstandings, I would like to provide a more detailed explanation of this issue.

To be clear, I'm not criticising your code or explanations! 😅 I think they're good, but the code with have today (written by many people) made the whole flow hard to grasp, so that took me a while.

It may be a lack of thinking on my part, I don't know why making createServer return a proxy can change the dev server reference in the configureServer hook.

I mean that if createServer returns a proxy that only proxies API to the newer dev server. Then we can let _createServer be a lot simpler since it only needs to deal with its own let server: ViteDevServer = {... instance by itself. It doesn't need to know that it's server reference could mean a new dev server or anything. That's handled within createServer and restartServer only. Or in other words, we no longer need _setInternalServer and Object.assign. (Though this is just an idea)

[sapphi-red]

Thanks for the PR!
I also vote to merge this PR for 5.1. If we don't get any reports about the incompatibility using Proxy, then I think we can try @bluwy's idea with more confidence.