cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.69k stars 702 forks source link

🐛 BUG: `crons = []` in `wrangler.toml` doesn't remove cron triggers #5450

Closed risu729 closed 4 months ago

risu729 commented 7 months ago

Which Cloudflare product(s) does this pertain to?

Wrangler core

What version(s) of the tool(s) are you using?

3.41.0

What version of Node are you using?

21.6.1

What operating system and version are you using?

WSL2 (Ubuntu 22.04.4 LTS)

Describe the Bug

Observed behavior

Cron triggers are not removed even if crons = [] set in wrangler.toml.

Expected behavior

Cron triggers are removed as documented here. https://developers.cloudflare.com/workers/configuration/cron-triggers/#via-wranglertoml-1

Steps to reproduce

See the minimal reproduction git repo.

How to reproduce the bug:

  1. npx wrangler deploy --triggers "* * * * *"
  2. npx wrangler deploy

If you see the triggers tab in the dashboard, * is still there.

Please provide a link to a minimal reproduction

https://github.com/risu729/wrangler-crons-test

Please provide any relevant error logs

No response

risu729 commented 7 months ago

I tested with wrangler@0.39.0, the version before #5426 was merged, but nothing changed.

I guess this line is suspicious. I'm not sure but it should run even if triggers.length is 0. https://github.com/cloudflare/workers-sdk/blob/638387e195f6aa54e7ed411274b17b7d52566663/packages/wrangler/src/triggers/deploy.ts#L218

RamIdeas commented 4 months ago

wrangler deploy (and wrangler triggers deploy, which just hoists the same logic from wrangler deploy) is only additive at this time. New triggers are created but existing triggers are not removed. This is because we do not want to remove triggers created in the dashboard that have not also be added to your wrangler.toml – please use the dashboard to remove existing triggers.

risu729 commented 4 months ago

Thanks for your reply! I got the current behavior. I opened an issue in docs to reduce confusion.