trunk-rs / trunk

Build, bundle & ship your Rust WASM application to the web.
https://trunkrs.dev/
Apache License 2.0
3.53k stars 255 forks source link

Re-thinking configuration #780

Open ctron opened 6 months ago

ctron commented 6 months ago

Right now there's a layer of configurations (order of priority, former will override latter):

The result is then passed on internally to structs for processing.

There are some areas of improvement:

  1. It doesn't always make sense to have the same behavior between CLI and config.
  2. CLI options sometimes influence how the configuration will be read
  3. environment variables get pushed into serde model, but actually are closer to the CLI model
  4. there are some inconsistencies (proxies, boolean flags not overriding false)

There are also some pending feature requests which seem to have an impact on the configuration:

  1. Profiles: It would be great if trunk would support build profiles. However, I would not really make sense to map those profiles to the CLI/env-vars. Only the selection should go there. However the profile selection should not be part of the serde configuration (also see: https://github.com/trunk-rs/trunk/issues/605).
  2. There was a discussion about adding the configuration to the Cargo.toml (personally I am not a fan of this, bet maybe we can have it both ways)
  3. Some parts of the build might want to have additional configuration (like minification level for CSS)
  4. There's the idea of plugins (has not manifested yet) But how could a plugin have its own configuration?

Bonus points:

  1. Allow other formats, beside TOML (I am looking at YAML). I would not force YAML on anyone, but with YAML it would be possible to leverage JSON schemas to enable IDE editor support.
simbleau commented 6 months ago

Why not eliminate the Trunk.toml entirely for Cargo.toml environment configuration? It serves virtually the same purpose, but the Trunk.toml adds noise to the root.

Other, mature bundlers, such as cargo-lambda for AWS, use this approach.

See https://www.cargo-lambda.info/commands/watch.html#environment-variables

For example, the Cargo.toml would look like this:

[package]
name = "basic-app"

[package.metadata.trunk.env]
RUST_LOG = "debug"
CARGO_PROFILE = "release"

[package.metadata.trunk.serve]
TRUNK_SERVE_ADDRESS = "127.0.0.1"
TRUNK_SERVE_PORT = 8080

I also think this fits better into the bevy ecosystem. In addition, environment variables are notoriously hard to work with in WASM, since they don't exist, but are still needed typically for things like deployments/environments, e.g. dev/stg/prod.

Today, if you want to have real deployment config for WASM apps, you have essentially three options:

Static:

pub const ENVIRONMENT = "dev";

Semi-static:

Fully dynamic, but requires features and special handling:

#[cfg(feature = "prod")]
pub const ENVIRONMENT = "prod";
#[cfg(feature = "stage")]
pub const ENVIRONMENT = "stg";
#[cfg(feature = "dev")]
pub const ENVIRONMENT = "dev";
ctron commented 6 months ago

Why not eliminate the Trunk.toml entirely for Cargo.toml environment configuration? It serves virtually the same purpose, but the Trunk.toml adds noise to the root.

I personally see I right the opposite way. I think having "trunk" config in a "trunk config file" makes more sense, rather than adding noise to the "cargo config". And there are other examples that go exactly that way (cargo-embed, https://probe.rs/docs/tools/cargo-embed/#configuration).

So I definitely would not want to force people into either direction. If possible, enable the use of Cargo.toml, but not force it.

When talking about env-vars, that was more about the configuration, less about the actual code. Right now, instead of using CLI arguments, one can use env-vars to provide that configuration to trunk. I think that comes in handy when talking about CI. But I also think it is not super valuable on all cases. Then again, I just might not know what people use. So a zero effort approach (like clap provides) makes sense to me. People can use it, if it makes sense to them.

For having those deployment style options, I think it makes sense to go into the direction of "build profiles". Something similar to cargo profiles:

[profile.release]
minify = true
features = ["foo"]
[profile.release-stage]
inherits = "release"
minify = true
features = ["foo", "stage"]

That could be aligned with cargo profiles too. So that enabling profile release-stage would activate cargo profile release-stage too, unless overridden.

simbleau commented 6 months ago

re: Cargo.toml config - Personally I see more people using the Cargo.toml than a bespoke file. I think it should've been the default from the beginning, personally.

Examples:

I think we're the odd man out, or at the least should support Cargo.toml configuration.

For having those deployment style options, I think it makes sense to go into the direction of "build profiles". Something similar to cargo profiles:

[profile.release]
minify = true
features = ["foo"]
[profile.release-stage]
inherits = "release"
minify = true
features = ["foo", "stage"]

That could be aligned with cargo profiles too. So that enabling profile release-stage would activate cargo profile release-stage too, unless overridden.

Are cargo profile features a thing? I'm not seeing any documentation for profile features. A stack overflow issue 3 years ago suggests this isn't allowed, either. However if it does exist, I would be happy with that! #605 would solve my problems with multi-stage environments.

ctron commented 6 months ago

Are cargo profile features a thing?

Not from cargo, but that would be the part the trunk would add. Still, selecting a trunk profile might also trigger the selection of a cargo profile. Maybe as opt-in, to not require a cargo profile for using trunk profiles.

I think when it comes to Cargo.toml vs Trunk.toml, you can find may examples for each side. But let's take a look at cargo-cross for example: https://github.com/cross-rs/cross?tab=readme-ov-file#configuration … It just gives you a bunch of options, and let's the user decide. And I think this is best approach. What's the least appealing option to me would be to force people into rewriting their existing Trunk.toml. Having tutorials out there which mention Trunk.toml, that would no longer be supported.

simbleau commented 6 months ago

Let's plan to support both.

As far as the cargo features go, I want to avoid confusion. Anything that would exist in the Trunk.toml should preface with package.metadata.trunk,

// Cargo.toml
[profile.release]
minify = true

[package.metadata.trunk.profile.release]
features = ["prod"]

[profile.release-stage]
inherits = "release"
minify = true

[package.metadata.trunk.profile.release-stage]
features = ["stage"]

On the Trunk.toml, though, we don't have to do that:

// Cargo.toml
[profile.release]
minify = true

[profile.release-stage]
inherits = "release"
minify = true
// Trunk.toml
[profile.release]
features = ["prod"]

[profile.release-stage]
features = ["stage"]
ctron commented 6 months ago

Yup, that sounds great. Personally I would also be keen on seeing:

# Trunk.yaml
$schema: "trunkrs.dev/config.schema.json"
profile:
  release:
     features: ["prod"]
  release-stage:
     features: ["prod"]

Assuming that's added by only a few lines of extra code. But the focus, for sure, should be on Cargo.toml and Trunk.toml.

ctron commented 6 months ago

Thinking about "when", I would prefer to finish up 0.20.0 first, and then work on that for 0.21.x? Would that work for you?

simbleau commented 6 months ago

Yup, that sounds great. Personally I would also be keen on seeing:

# Trunk.yaml
$schema: "trunkrs.dev/config.schema.json"
profile:
  release:
     features: ["prod"]
  release-stage:
     features: ["prod"]

Assuming that's added by only a few lines of extra code. But the focus, for sure, should be on Cargo.toml and Trunk.toml.

I'd prefer a version: 1 or something in the YAML, since the URL contents can change.

simbleau commented 6 months ago

Thinking about "when", I would prefer to finish up 0.20.0 first, and then work on that for 0.21.x? Would that work for you?

Sure, this feels like a great change. Probably the highest on my priority list, among workspace support.

ctron commented 6 months ago

Yup, that sounds great. Personally I would also be keen on seeing:

# Trunk.yaml
$schema: "trunkrs.dev/config.schema.json"
profile:
  release:
     features: ["prod"]
  release-stage:
     features: ["prod"]

Assuming that's added by only a few lines of extra code. But the focus, for sure, should be on Cargo.toml and Trunk.toml.

I'd prefer a version: 1 or something in the YAML, since the URL contents can change.

Well, that would not allow editor support. And that's the whole idea behind that schema. Being able to auto-complete configurations.

simbleau commented 6 months ago

Yup, that sounds great. Personally I would also be keen on seeing:

# Trunk.yaml
$schema: "trunkrs.dev/config.schema.json"
profile:
  release:
     features: ["prod"]
  release-stage:
     features: ["prod"]

Assuming that's added by only a few lines of extra code. But the focus, for sure, should be on Cargo.toml and Trunk.toml.

I'd prefer a version: 1 or something in the YAML, since the URL contents can change.

Well, that would not allow editor support. And that's the whole idea behind that schema. Being able to auto-complete configurations.

Sounds good- We could do trunkrs.dev/config.schema.v1.json then?

ctron commented 6 months ago

Btw, I am slowly working towards this (as time permits). There's nothing to share yet, but I'll share a PR as soon as there is something that compiles.

ctron commented 5 months ago

There's an initial draft PR. It's not finished and lacks Cargo.toml support. Although that's prepared. If you're curios, maybe take a look at https://github.com/trunk-rs/trunk/pull/795 … if want to try out something, maybe wait a bit longer ;)

simbleau commented 5 months ago

Loving the progress @ctron ! This has, by far, been the most requested thing I've wanted for years. :)

NiklasEi commented 4 months ago

Can this be closed since #795 was merged?

ctron commented 4 months ago

I am not 100% sure where are there yet. So the answer is: maybe :) But let's keep it open until it's released and everyone (mostly everyone … some people) are happy.

benfrankel commented 4 months ago

I think https://github.com/trunk-rs/trunk/issues/605 is not yet fixed by https://github.com/trunk-rs/trunk/pull/795?

simbleau commented 3 months ago

I think #605 is not yet fixed by #795?

That's right, the work @ctron did to re-work configuration was for configuration maintainability.

I believe #719 and #605 is still much needed quality of life, and planned, but I don't have bandwidth to support that development at the moment.