A way to instantiate a resource that must be a singleton inside a dev environment

[sapphi-red]

Description

I noticed that we have a same problem with #12734 if a different server (e.g. miniflare) is used in DevEnvironment.
When restarting the server (by editing the config or pressing r + enter), the next instantiation happens before devEnv::close is called. In the end the server crashes with address already in use error caused by the different server.
#16358 (comment)

configureServer has the same problem: #17285

Suggested solution

Made two PRs with different approach.

• feat: add isRestart for environment #18262 feat: add environment::restart #18353
• feat: add environment::listen #18263

Alternative

No response

Additional context

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

[hi-ogawa]

From a quick look, listen seems easier to understand. To utilize isRestart, wouldn't it still require keeping things on globalThis because environments and plugins are re-instantiated? I'm not sure what's intended, but I can only think of something fairly ugly:

class MyEnv extends DevEnvironment {
  init({ isRestart }) {
    if (isRestart) {
      // recreate it here
      globalThis.__something.close()
      globalThis.__something = new SomeResource(someOptions)
  
      // or maybe resource can restart itself
      // globalThis.__something.restart(someOptions)

    } else {
      globalThis.__something = new SomeResource(someOptions)
    }
  }

  // then what to do here?
  // close() {}
}

For listen, I guess I can just put it under instance:

class MyEnv extends DevEnvironment {
  public resource;
  listen() {
    this.resource = new SomeResource(someOptions)
  }
  close() {
    this.resource.close();
  }
}

If I have listen and also know that init works unintuitively, I think I would move all "init" logic to listen. Would it limit anything for environment usages? If not, then is it possible to rename current init as _initInternal and use proposed listen as new init?

[sapphi-red]

Oh, that's right. @patak-dev Did you have something in mind that resolve this problem for the isRestart flag?

[patak-dev]

I wasn't thinking on a isRestart flag, but instead in having a environment.restart() function like we have for the server.restart(). Instead of creating a new environment, if there was an environment with the same name, then that instance will get a environment.restart(newConfig) and it could decide to keep the state it doesn't want to re-create. This may be quite hard to implement (if it is possible), so if listen() gives us a way to somehow avoid the need to destroy and re-create the world on restart for certain environments, then that sounds good to me. I wonder if we still should have some concept like environment.init(prevEnvironmentInstance) for environments to adopt whatever they need from the instance that is about to get destroyed.

[sapphi-red]

I see. Yeah, I think we need a way to take over the previous state (e.g. using the same port and not finding a random port again).

[sapphi-red]

I made a PR that implements environment.restart (#18353). Also changed #18263 to make it possible to get the previous env instance.

[patak-dev]

I made a PR that implements environment.restart (#18353). Also changed #18263 to make it possible to get the previous env instance.

It isn't easy for me to decide which approach is best. I'm tempted to include both of them as experimental and let the ecosystem play with them. Worse case we deprecate one later on. Do you have a preference here?

[sapphi-red]

I slightly prefer #18263. It feels easier to understand for me.

class MyDevEnvironment {
  #server: MyServer

  async init({ options, previousInstance }) {
    const port = previousInstance?.server.port ?? 5179

    this.#server = createServer(port, this.config)
  }

  async listen() {
    await this.#server.listen()
  }

  async close() {
    await this.#server.close()
  }
}

class MyDevEnvironment {
  #server: MyServer

  async init({ options }) {
    const port = previousInstance?.server.port ?? 5179

    this.#server = createServer(port)
    await this.#server.listen()
  }

  async restart(newConfig) {
    const port = this.#server.port
    await this.#server.close()
    this.#server = createServer(port, newConfig)
    await this.#server.listen()
  }

  async close() {
    await this.#server.close()
  }
}