Packages / Env: Allow cast file or default to string

[joselcvarela]

Scope

It is possible to pass a file to env by using <CONFIG>_FILE and the value should be the location of the file.

Currently this file is sometimes being recognized as array, json or any other type as the value is always casted and the type is guessed when there are no more information.

In this PR we:

• allow file to be casted to another type like array or json
• recognize file as string by default

What's changed:

• File is recognized as string by default
• File can be casted as any other type

Potential Risks / Drawbacks

• If someone is relying on having their file being recognized as a random unexpected type, they can have issue

Review Notes / Questions

• N/A

[changeset-bot]

⚠️ No Changeset found

Latest commit: e5041d8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[paescuj]

recognize file as string by default

@joselcvarela Not sure about this point, since it would introduce different behavior for _FILE variables vs. "plain" variables, and in particular that type mapping would no longer apply to files.

allow file to be casted to another type like array or json

I understand the need for this, though 👍 Would it be sufficient to just implement this point? Then it would also not be a breaking change.

[br41nslug]

Is it an option here to just make sure files are always read as string so the client can reliably cast/parse/format it into the expected format. I would love to reduce the amount of automagic casting instead of increase it.

[paescuj]

IMO _FILES variables should just behave the same as their non-_FILES counterpart, so that we neither have to add nor remove any (auto)casting.

[br41nslug]

@paescuj So what exactly are you suggesting? Either they currently already do behave the same right (even tho there is no real defined behavior for something like TEST_FILE=array:somefile.txt,otherfile.txt) or Jose's change will make them behave the same in the expected parsed value.

[paescuj]

@paescuj So what exactly are you suggesting? Either they currently already do behave the same right (even tho there is no real defined behavior for something like TEST_FILE=array:somefile.txt,otherfile.txt) or Jose's change will make them behave the same in the expected parsed value.

Yeah, applying the following change from this PR

allow file to be casted to another type like array or json

while dropping this one

recognize file as string by default

[br41nslug]

Ah so you want to keep the type guessing when not explicitly casting the type then?

As my first comment said my preference would definitely go out to relying less on the guessType utility and have more reliable/explainable types when dealing with env vars.

The long term goal for environment variables is reduce the reliance on casting and eventually have them all be treated as string and only explicitly casted/parsed by the end-user/logic where needed (which is what we've slowly been doing by using more and more Boolean(env['SOME_VAR']) in our codebase instead of casting the var at the top level). Perhaps @rijkvanzanten can chime in whether that is still the vision for the future?

[paescuj]

Ah so you want to keep the type guessing when not explicitly casting the type then?

Exactly, I don't see why we should treat _FILE variables different than the other variables.

As my first comment said my preference would definitely go out to relying less on the guessType utility and have more reliable/explainable types when dealing with env vars.

The long term goal for environment variables is reduce the reliance on casting and eventually have them all be treated as string and only explicitly casted/parsed by the end-user/logic where needed (which is what we've slowly been doing by using more and more Boolean(env['SOME_VAR']) in our codebase instead of casting the var at the top level). Perhaps @rijkvanzanten can chime in whether that is still the vision for the future?

Yeah, I agree and also think that's still the goal. But I'd rather address that separately. It should be sufficient for now to allow _FILE variables to be force-casted. This way it also wouldn't be a breaking change.