Skip to content

feat: resolve server URLs before calling other listeners#19981

Merged
sapphi-red merged 4 commits intovitejs:mainfrom
sirdiego:sync_server_start
Jul 25, 2025
Merged

feat: resolve server URLs before calling other listeners#19981
sapphi-red merged 4 commits intovitejs:mainfrom
sirdiego:sync_server_start

Conversation

@sirdiego
Copy link
Contributor

@sirdiego sirdiego commented May 1, 2025

Description

This PR moves the resolving of the URLs to before the server is started. This way one can use configureServer to get those information from the server.

      configureServer(server) {
        server.httpServer.on('listening', () => {
          console.log(server.resolvedUrls)
        })
      },

I added a first test but am a little lost in how and where I could test the integrated functions. I'd love for this to get merged but would also appreciate help on how to write more tests here.

Thank you folks so much!

@sirdiego sirdiego changed the title feat: Resolve server URLs before calling other listeners feat: resolve server URLs before calling other listeners May 1, 2025
@sapphi-red sapphi-red added p2-nice-to-have Not breaking anything but nice to have (priority) feat: dev dev server labels May 2, 2025
@sapphi-red
Copy link
Member

I added a first test but am a little lost in how and where I could test the integrated functions. I'd love for this to get merged but would also appreciate help on how to write more tests here.

I think you can put it in packages/vite/src/node/__tests__/dev.spec.ts with something like

test('check', () => {
  expect.assertions(1)
  const { promise, resolve } = promiseWithResolvers<void>()
  const server = await createServer({
    root: __dirname,
    logLevel: 'error',
    server: {
      port: 9400, // make sure the port is unique
      ws: false,
    },
    plugins: [/* add plugin that checks server.resolvedUrls here */] 
  })
  await server.listen()
  await promise
})

It needs to wait for the configureServer hook to be called so using promiseWithResolvers is needed.
https://github.com/vitejs/vite/pull/19936/files#diff-0244b1dfcfa697a921ae7858a6beb979114a8c59b478ff360e72fdeb82b0b82cR226-R247

@sirdiego
Copy link
Contributor Author

sirdiego commented May 2, 2025

I added a first test but am a little lost in how and where I could test the integrated functions. I'd love for this to get merged but would also appreciate help on how to write more tests here.

I think you can put it in packages/vite/src/node/__tests__/dev.spec.ts with something like

test('check', () => {
  expect.assertions(1)
  const { promise, resolve } = promiseWithResolvers<void>()
  const server = await createServer({
    root: __dirname,
    logLevel: 'error',
    server: {
      port: 9400, // make sure the port is unique
      ws: false,
    },
    plugins: [/* add plugin that checks server.resolvedUrls here */] 
  })
  await server.listen()
  await promise
})

It needs to wait for the configureServer hook to be called so using promiseWithResolvers is needed. https://github.com/vitejs/vite/pull/19936/files#diff-0244b1dfcfa697a921ae7858a6beb979114a8c59b478ff360e72fdeb82b0b82cR226-R247

Thanks for the suggestion. That worked out very good. I added a new suite to the dev.spec.ts that fails without this patch and is green with this patch.

sapphi-red
sapphi-red previously approved these changes May 7, 2025
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@sirdiego
Copy link
Contributor Author

sirdiego commented May 7, 2025

Has the Windows test fail anything to do with these changes? Is there something I can do here?

@sapphi-red
Copy link
Member

It's probably a flaky fail. I've retriggered the CI 👍

@sapphi-red sapphi-red added this to the 7.1 milestone Jun 26, 2025
@sapphi-red sapphi-red merged commit 45f6443 into vitejs:main Jul 25, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: dev dev server p2-nice-to-have Not breaking anything but nice to have (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants