feat(cloudflare): add support for wrangler deploy env

[dario-piotrowicz]

🔗 Linked issue

❓ Type of change

• [ ] 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
• [ ] 🐞 Bug fix (a non-breaking change that fixes an issue)
• [ ] 👌 Enhancement (improving an existing functionality like performance)
• [ ] ✨ New feature (a non-breaking change that adds functionality)
• [ ] 🧹 Chore (updates to the build process or auxiliary tools and libraries)
• [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

📝 Checklist

• [ ] I have linked an issue or discussion.
• [ ] I have updated the documentation accordingly.

[dario-piotrowicz]

This should also work in dev, so we might want to include it in https://github.com/nitrojs/nitro-cloudflare-dev? 🤔

but maybe not since nitro-cloudflare-dev will become unnecessary in nitro v3, so we just need to make sure this applies in nitro v3 both for dev and build, right? 🤔

[dario-piotrowicz]

I've checked with the rest of the Cloudflare team and the general feeling is to implement a solution so that nitro can import a utility for the environment resolution from wrangler (or a different new workers-sdk package) instead or reimplementing the logic as I am doing there.

The team agreed on waiting for such utility to be available instead of trying to progress with the current solution in this PR.

When the utility is ready I will update this PR accordingly 🙂

So anyways this PR will likely not be worked on for a bit until the utility is implemented ready to be used (ballpark ETA I'd say maybe a month or so, possibly sooner).

Note: as soon as we will have some public issue to track this work in the workers-sdk I will share it here 🙂

[pi0]

@dario-piotrowicz any update on this 🙇🏼

[dario-piotrowicz]

Hey @pi0 👋 I'm so very sorry for this being stalled for months! 🙇 (it came back to mind every now and then but I've always had other stuff prioritized before it)

I am planning to start working on the changes in wrangler next week, I hope that's ok, I will let you know as soon as they are updates 🙇

[dario-piotrowicz]

@pi0, first step for addressing this 🙂

https://github.com/cloudflare/workers-sdk/pull/10044

If you'll have a sec I'd love it if you'd have a quick look at the PR and let me know if this is heading in the right direction from your point of view 😄

Especially fixtures/third-party-tool-config should be a good example for you 🙂

[pi0]

Thanks for the update, dear @dario-piotrowicz!

readConfig/unstable_readConfig looks promising, but the issue is, Nitro build with cloudflare preset should work without an external dependency on wrangler as we are trying to reduce dependencies as much as possible.

How do you see the next step for us here? redist might work but looking at src/config in wrangler, there are some deps like find-up, ts-dedent, "@iarna/toml, jsonc-parser, @iarna/toml.

Are you up to reduce this external dependencies little bit? Most notably, i think you could migrate to confbox for most of config readers.

[dario-piotrowicz]

redist might work but looking at src/config in wrangler, there are some deps like find-up, ts-dedent, "@iarna/toml, jsonc-parser, @iarna/toml.

The dependencies would still be there but bundled into the wrangler/config entrypoint, so basically I am trying to make wrangler/config as lean as it can be but without reimplementing everything from scratch (as you can see from the fixture, the wrangler/config could be bundled into a tool such as nitro without bringing in the heavy miniflare/workerd dependencies), is this not ok for you? Did you have in mind a different solution?

How do you see the next step for us here?

The idea was to land this change, then form there:

• expose the config typescript types (that nitro needs)
• split the readConfig logic in the two:
  • environment-specific config flattening (the one nitro could use)
  • config default resolution (which nitro would not care about)

And I thought that that would be good enough for nitro since:

• the logic nitro would have to bundle in would be significantly reduced
• the typescript types would be exposed
• the flattening functionality nitro is interested in would be exposed

Are you up to reduce this external dependencies little bit? Most notably, i think you could migrate to confbox for most of config readers.

I can check with the team and see if I can allocate more time on this, but as for the current roadmap I don't think I have much time to reduce the dependencies and that that would have to ideally be done later

[dario-piotrowicz]

Nitro build with cloudflare preset should work without an external dependency on wrangler as we are trying to reduce dependencies as much as possible.

Could you clarify what you mean by "should work without an external dependeny on wrangler"?

I thought you could import and bundle in nitro the content of wrangler/config (so nitro would have wrangler as a dev dependency), it admittedly is not super lean but I'd hope lean enough?

I might have misunderstood the requirements there, please let's clarify what nitro would need now since at this early stage I can still somewhat easily change course 🙂

[pi0]

Yes as long as you are ok with rebundling wrangler/config it should be fine.

My only concern now is additional pkg size overhead for nitro if we do bundle. I guess might be easier if you keep wrangler dependencies as externals we might find a way to rewrite them into shared deps like confbox.

Alternatively we can have a "sync" script to sync contents of wrangler/src it might be most straight forward strategy here and i think less demanding for your side. How does that sound?

[dario-piotrowicz]

Yes as long as you are ok with rebundling wrangler/config it should be fine.

Ok, so I am fine continuing going this direction? 🙂

My only concern now is additional pkg size overhead for nitro if we do bundle. I guess might be easier if you keep wrangler dependencies as externals we might find a way to rewrite them into shared deps like confbox.

mh... yes that would be possible, but I wonder how that could work for other consumers of the API, having the dependencies externals can cause issues since consumers would generally need to install the dependencies themselves right? I don't know how I can justify including such a behavior in wrangler if it's only geared at targeting nitro (as I would imagine that most consumers would just expect it to work out of the box and not have to deal with any dependencies or rewrite?)

Alternatively we can have a "sync" script to sync contents of wrangler/src it might be most straight forward strategy here and i think less demanding for your side. How does that sound?

Sorry I don't follow the suggestion, what would this "sync" script do?

[pi0]

Wrangler has already couple of dependencies in package.json, i was suggesting to also list these there.

For alternative, a sync script here can bundle wrangler/src/config/* from git upstream instead of depending on npm package that has issues like dependency bundling. Since anyway we are doing a redist, and if main concern here is to not maintain logic inside nitro, we could simply just sync (automated) and copy from upstream instead of rebundling.

[dario-piotrowicz]

Wrangler has already couple of dependencies in package.json, i was suggesting to also list these there.

I see, yeah might not be a huge deal 🤔

For alternative, a sync script here can bundle wrangler/src/config/* from git upstream instead of depending on npm package that has issues like dependency bundling. Since anyway we are doing a redist, and if main concern here is to not maintain logic inside nitro, we could simply just sync (automated) and copy from upstream instead of rebundling.

I see, yeah that does sound like a good alternative solution, but it also sounds pretty complex, error prone and potentially costly to maintain 🤔 I think that I'd indeed opt for externalizing the dependencies then

[pi0]

also sounds pretty complex, error-prone

Happy to give it a try at least for types it should make it easier i imagine also less complex than rebundling optimization and asking you to keep some as external deps. (nitro presets are file-to-file transform currently not bundle)

To avoid regressions, we can have a simple unit test to verify that after sync, readConfig util works as expected.

[dario-piotrowicz]

also sounds pretty complex, error-prone

Happy to give it a try at least for types it should make it easier i imagine also less complex than rebundling optimization and asking you to keep some as external deps. (nitro presets are file-to-file transform currently not bundle)

To avoid regressions, we can have a simple unit test to verify that after sync, readConfig util works as expected.

Ok thank you 🙂

I'll see how externalizing dependencies would look like and if devs on the team are ok with that, we can then decide from there how to proceed if that sounds good to you 😄

[pi0]

Surely. I just did a quick try and noticed there are path imports (outside packages/wrangler/src/config) like ../logger and @cloudflare/workers-shared dependency as well, maybe rebundling might work better, or if somehow making /config src better isolated from wrangler or be in standalone @cloudflare/config?

(will wait for your update)

---

sync script for my future reference only:

import { downloadTemplate } from "giget";
import { fileURLToPath } from "node:url";

await downloadTemplate(
  "gh:cloudflare/workers-sdk/packages/wrangler/src/config#dario/DEVX-2130/config-entrypoint",
  {
    forceClean: true,
    dir: fileURLToPath(
      new URL("../src/presets/cloudflare/wrangler/config", import.meta.url)
    ),
  }
);

[dario-piotrowicz]

Surely. I just did a quick try and noticed there are path imports (outside packages/wrangler/src/config) like ../logger and @cloudflare/workers-shared dependency as well, maybe rebundling might work better, or if somehow making /config src better isolated from wrangler or be in standalone @cloudflare/config?

For sure, /config is not well isolated at all... I can try to isolate it more but given how it depends on other code (like the logger) that might be a bit tough to do 😓

@cloudflare/config would have the same issue, but also the team wasn't super keen on that since there was a general agreement that adding an extra package would add too much maintenance burden on our side 😓

[dario-piotrowicz]

@pi0, could you have another look at my PR and let me know what you think? 🙏 https://github.com/cloudflare/workers-sdk/pull/10044

I've make most of the dependencies that make it into /config external

The CI on the PR is also green, so if you're happy with this first iteration I can start asking for reviews and see if we can land this 🙂 (next I'll work on the other steps I mentioned above)