unstable_dev() -> dev()

[rozenmd]

We've agreed that further plans (a potential Events API for running wrangler dev programmatically) won't be breaking changes, so won't block us renaming unstable_dev() to dev()

TODO:

• Finalize Public API - Pages has chucked some options into it that we might not want public
• ???

[rozenmd]

CC: @JacobMGEvans @penalosa @GregBrimble @mrbbot

[GregBrimble]

I think an experimental object would be useful to add (even if we don't really have anything in it to start with). TailwindCSS and Next.js are pretty good examples. This will allow us to try out new options without promising any public contract.

[penalosa]

I can PR that if you're amenable?

[GregBrimble]

scriptPath in case we offer script as a string in the future?

[penalosa]

I feel like script for consistency with wrangler.toml and the CLI options, and maybe source/content for script content in the future?

[rozenmd]

Keen for it @penalosa!

[rozenmd]

Alright this has been shipped

[GregBrimble]

Perhaps should require absolute paths if we're accepting script paths/config paths until we firm up how we expect relative paths to work? @penalosa said something about them being a bit weird today (relative to package.json or something?)

[penalosa]

I mean, can't you use ${process.cwd()}/src/index.ts if you really want it to be shareable?

[penalosa]

But I do think the relative behaviour should be relative to where it's called (essentially, I think it should be identical to fs)

[rozenmd]

Maybe, but it'd feel like magic if you could just do const worker = await unstable_dev('./index.ts')

[GregBrimble]

/Users/rozenmd/Testing/my-project/src/index.ts

Ah, gotcha gotcha. Yeah, I think that's bad.

Not quite sure what you're meaning in your const worker = await unstable_dev('./index.ts') example. Where is that being run? How is that better than:

- tests
  - index.test.ts
- src
  - index.ts

Where index.test.ts looks like:

unstable_dev('../src/index.ts')

Perhaps we could also (in the future) support looking for tsconfig.json path aliases e.g.

unstable_dev('~/index.ts')

where

- tsconfig.json
- tests
- src

and

{
  "compilerOptions": {
    "paths": {
      "~": "./src"
    }
  }
}

or however that works... you get the gist.

[rozenmd]

Yeah @GregBrimble that's basically the same example (compared to now, where it would be src/index.ts and it's unintuitive)

[GregBrimble]

Also should commit to how we want this thing to be stopped. I know we return a stop() function. Is that sync/async? If I exit this Node.js process that calls dev() without first calling stop(), what do I want to happen?

[rozenmd]

stop() has to be async because we're shutting down the http server via http-terminator, an async fn

[GregBrimble]

What's the default port story right now? Try 8787 and error if unavailable? Do we want to do that? Or fallback to 0?

Do we even want the dev() API to default to 8787? Is there a world where it's reasonable to run the JS API alongside the CLI? Do we want to default to some other number for the JS API?

[GregBrimble]

Maybe there's value in a headless option where we either just don't report back the server info, or where we figure out how to spin up a server without ever listening on the port and all requests must come through Worker.fetch()

[penalosa]

What about just defaulting to 0 if nothing is passed in? That seems honestly pretty reasonable for something like this? (although, taking inspiration from http's listen (and express etc...), maybe we require a port to be passed in, and hard error if it's not free. That might be the better expectation for a library like this?

[GregBrimble]

+1 to hard error if a port is explicitly requested and unavailable.

Defaulting to 0 seems reasonable but I'm not totally sold. Maybe I need to look at prior art some more.

[mrbbot]

If we default to 0, and hard error if a specified port is taken, we'll have a hard time implementing Wrangler's CLI on top of dev().
For that, we'd want to pass port 8787, but fallback to a different one if that was taken. We could implementing this by checking if 8787 were free first, but that would introduce a race. I'd probably go with defaulting to 8787 and fallback to random if that fails to bind, then hard error if a specified port was taken. Or we could add a config option for controlling the fallback behaviour?

[rozenmd]

At the moment, we default to 0 if nothing is passed in. If a port is provided via config or flags, and it isn't available, we throw an error. It isn't a hard error though, wrangler dev keeps running 😕

[mrbbot]

I think we should only accept a single options object in dev(), it should be possible to collapse options and apiOptions here: https://github.com/cloudflare/wrangler2/blob/bbfb6a960e1f57a1b3214497f05f1d55b8dfb5c0/packages/wrangler/src/api/dev.ts#L80-L81
To be honest, if we had a React free dev, we could remove testMode, and presumably we'll also be removing disableExperimentalWarning, so apiOptions could just be removed.

[rozenmd]

Yeah I agree, it probably should just be options

For context:

• testMode was a hack for not having enough time to implement watching/rebuilding+refreshing the preview Worker when code changes
  • to implement a React-free dev we'd probably want a state machine to model dev's data flows and state

[rozenmd]

This has been shipped