Schema: containerEnv's type should add support for "boolean"

[samruddhikhandale]

          @samruddhikhandale maybe a bit nitpicky but I would like to be able to use JSON booleans in `containerEnv`, validation is failing because only strings are allowed.

          ``` json
          {
              "containerEnv": {
                  "DOTNET_RUNNING_IN_CONTAINER": true
              }
          }
          ```

Originally posted by @sliekens in devcontainers/features#628 (comment)

[avidal]

It feels like this is an unnecessary complication, it leaves it up to implementing tools to decide how to render a JSON boolean (unless I suppose the spec takes a hard stance on it). Generally speaking, tools will either use the presence of a certain environment variable to indicate "true" or a set of specific values (such as "1", "true", "yes", "on").

If I'm reading the original issue correctly, all that matters is that it's present with any value which seems entirely consistent with leaving the spec alone?

Is there a significant UX benefit to changing the spec? My concern is that another tool wants a specific value to indicate truthiness and now you have containerEnv entries with both true and "true" which seems to be more confusing (in a case where true renders as, say, "1").

[sliekens]

I originally raised this because I expected to be able to use JSON types in JSON configuration.

Sorry, but having to write this feels really weird:

{
  "containerEnv": {
    "FLAG": "true"
  }
}

versus

{
  "containerEnv": {
    "FLAG": true
  }
}

This is what you intuitively write when you are typing JSON code so it's an annoyance that it doesn't work (crashes dev container cli).

I think boolean true should just be stringified and render the string "true".

[avidal]

I think boolean true should just be stringified and render the string "true".

Yeah, sure. I mean, I don't have a strong objection to it. It's more that the enclosing type is clearly about environment variables and environment variables must have string values, so any tool that accepts anything other than strings must take a stance on exactly how to render non-string values. It seems to me that the least surprising thing is to require users to specify strings.

Also, it doesn't look like the CLI crashes. It exits with a validation error. I do think it could use a more friendly error message though.

I originally raised this because I expected to be able to use JSON types in JSON configuration.

It's configuration though, and not code. The implementation decides what's valid and what isn't, no matter what tool you're using that consumes JSON.

---

Ultimately, I'm just someone building a (non JS) implementation of the spec and not an actual spec maintainer. My opinion is usually (except in cases where alternatives provide significant UX improvements) going to be biased towards ease of implementation and in every case where a particular field which currently supports multiple types can be reduced to a single type I'm going to prefer that choice because it makes my implementation simpler :)

[sliekens]

I just checked again and VSCode does give a fair warning that containerEnv expects strings in devcontainer.json (but not in devcontainer-feature.json). And you are right, a friendlier error message instead of a technical error would not have caused me frustration. So perhaps adding a better validation message without relaxing the validation is where we can find middle ground.