Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 BUG: Routes in wrangler.toml doesn't respect env #513

Closed
istarkov opened this issue Feb 22, 2022 · 2 comments · Fixed by #517 or #484
Closed

🐛 BUG: Routes in wrangler.toml doesn't respect env #513

istarkov opened this issue Feb 22, 2022 · 2 comments · Fixed by #517 or #484
Assignees
@threepointone

Comments

@istarkov
Copy link

What version of Wrangler are you using?

0.0.17

What operating system are you using?

Mac, Linux

Describe the Bug

How to reproduce

Create different routes for production env in wrangler.toml file

routes = ["example-test.blabla.ch/*"]
[env.production]
routes = ["example.blabla.ch/*"]

execute

wrangler publish ./src/example.ts --env production --config ./wrangler.toml

Worker will be deployed to example-test.blabla.ch/* instead of example.blabla.ch/*.

This because here https://github.com/cloudflare/wrangler2/blob/9534c7fd1351daacaed63b3a3e2fafa884b515a8/packages/wrangler/src/publish.ts#L303 env params is not used at all (its used for vars, durable objects etc, but not for routes) see bindings above.
@istarkov istarkov added the bug label Feb 22, 2022
@threepointone
Copy link
Contributor

good catch, sorry about that! I'll fix this asap.

@threepointone threepointone self-assigned this Feb 22, 2022
@threepointone threepointone moved this to Must-have in workers-sdk Feb 22, 2022
@Electroid Electroid added this to the 2.0 milestone Feb 22, 2022
threepointone added a commit that referenced this issue Feb 22, 2022
@threepointone
fix: publish environment specific routes
ff3352e 
This adds some tests for publishing routes, and fixes a couple of bugs with the flow.
- fixes publishing environment specific routes, closes #513
- default workers_dev to false if there are any routes specified
- catches a hanging promise when we were toggling off a workers.dev subdomain (which should have been caught by the no-floating-promises lint rule, so that's concerning)
- this also fixes publishing environment specific crons, but I'll write tests for that when I'm doing that feature in depth

I expect to send some followup PRs for routes, but this should make it a little more stable right now
threepointone added a commit that referenced this issue Feb 22, 2022
@threepointone
fix: publish environment specific routes
b0190d0 
This adds some tests for publishing routes, and fixes a couple of bugs with the flow.
- fixes publishing environment specific routes, closes #513
- default workers_dev to false if there are any routes specified
- catches a hanging promise when we were toggling off a workers.dev subdomain (which should have been caught by the no-floating-promises lint rule, so that's concerning)
- this also fixes publishing environment specific crons, but I'll write tests for that when I'm doing that feature in depth

I expect to send some followup PRs for routes, but this should make it a little more stable right now
@threepointone threepointone mentioned this issue Feb 22, 2022
Merged
threepointone added a commit that referenced this issue Feb 23, 2022
@threepointone
fix: publish environment specific routes (#517)
201a6bb 
This adds some tests for publishing routes, and fixes a couple of bugs with the flow.
- fixes publishing environment specific routes, closes #513
- default workers_dev to false if there are any routes specified
- catches a hanging promise when we were toggling off a workers.dev subdomain (which should have been caught by the no-floating-promises lint rule, so that's concerning)
- this also fixes publishing environment specific crons, but I'll write tests for that when I'm doing that feature in depth
Repository owner moved this from Must-have to Done in workers-sdk Feb 23, 2022
@github-actions github-actions bot mentioned this issue Feb 23, 2022
Merged
@threepointone
Copy link
Contributor

This is fixed in wrangler@alpha, we'll do a @beta release by the end of the week. Thanks for reporting this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@threepointone threepointone

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@threepointone @Electroid @istarkov