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.

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

config.build should be inheritable #588

Closed
3 tasks done
threepointone opened this issue Mar 14, 2022 · 2 comments · Fixed by #673 or #663
Closed
3 tasks done

config.build should be inheritable #588

threepointone opened this issue Mar 14, 2022 · 2 comments · Fixed by #673 or #663
Assignees

Comments

@threepointone
Copy link
Contributor

threepointone commented Mar 14, 2022

config.build should be inheritable/redefinable across environments (which makes sense, because builds/build commands are typically different across environments; different vars, params, etc.). I think wrangler1 supports this.

This is a bit tricky because this might imply that main/rules should be inheritable. Maybe it should be. But it would be nice if this combination would work:

  • top level main
  • top level build
  • env specific build (that outputs to top level main)
  • and optionally env specific main

I spent a few minutes on this but got confused, so I'm filing this issue instead for now.

@threepointone threepointone moved this to Must-have in workers-sdk Mar 16, 2022
@petebacondarwin petebacondarwin self-assigned this Mar 17, 2022
@petebacondarwin petebacondarwin added this to the 2.0 milestone Mar 17, 2022
@petebacondarwin
Copy link
Contributor

I think making main an inheritable field makes sense. Even without a custom build I could imagine having:

src/
  index.dev.ts
  index.prod.ts

where various env specific constants etc could be defined.

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Mar 19, 2022

And we have already agreed for rules to be inheritable:

https://github.com/cloudflare/wrangler2/blob/6b44822b1742e2b84e6b38c383e78043db922b67/packages/wrangler/src/config/environment.ts#L126-L134

We have deprecated the rules that existed in the upload field:

https://github.com/cloudflare/wrangler2/blob/6b44822b1742e2b84e6b38c383e78043db922b67/packages/wrangler/src/config/config.ts#L261-L265

Although I note in the code, we are just grabbing the rules from the top-level environment and ignoring the current environment. I think this might be a problem across the codebase for other properties; or at least it is a potential foot-gun.
So I will put together a PR to make it more easy to do the right thing.

@petebacondarwin petebacondarwin moved this from Must-have to In Progress in workers-sdk Mar 19, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Mar 20, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Mar 20, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Mar 20, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Mar 21, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Mar 21, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Mar 22, 2022
…vironment"

Now the `build` field can be specified within a named environment, overriding whatever
may appear at the top level.

Resolves cloudflare#588
@petebacondarwin petebacondarwin moved this from In Progress to In Review in workers-sdk Mar 22, 2022
petebacondarwin added a commit that referenced this issue Mar 22, 2022
…vironment"

Now the `build` field can be specified within a named environment, overriding whatever
may appear at the top level.

Resolves #588
Repository owner moved this from In Review to Done in workers-sdk Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants