rogerluan / arkana

Use dotenv files for Android and iOS projects.
BSD 2-Clause "Simplified" License
379 stars 18 forks source link

Keys in different environment doesn't conform the same protocol for Debug and Release #70

Closed CarlosNano closed 1 month ago

CarlosNano commented 3 months ago

Hi

We got this .arkana.yml file:

environments:
  - Debug
  - Release
  environment_secrets:
  - HeapIOKey

The .env file has something like this: HeapIOKeyDebug="1234567890" HeapIOKeyRelease="4444400102"

But when we run arkana the swift files generated won't compile because heapIOKeyis a different type depending of the environment:

heapIOKey in Debug is an Int

public extension Keys {
    struct Debug: KeysEnvironmentProtocol {
        public init() {}

        @inline(__always)
        public let heapIOKey: Int = {

heapIOKey in Release is a String

public extension Keys {
    struct Release: KeysEnvironmentProtocol {
        public init() {}

        @inline(__always)
        public let heapIOKey: String = 

This won't compile because:

public protocol KeysEnvironmentProtocol {
    var heapIOKey: Int { get }
}

Is there a way to force arkana to have a particular type? Because I think the type should be a String in this case

rogerluan commented 3 months ago

Hi @CarlosNano 👋

Is there a way to force arkana to have a particular type?

Not currently, no.

Because I think the type should be a String in this case

I agree! And found your report quite interesting 😲 in your example for the .env file, both of your values happen to be number-only, but do the real values in your real .env file contain values that look like a number and the other look like a string? Like:

HeapIOKeyDebug="99999999999999"
HeapIOKeyRelease="AAAAAAAA99999"

Because that would explain the behavior observed. If not, idk what's causing this from the top of my mind (yet).

Just for a little bit of context for you: Arkana infers the type based on what the string looks like. Those .env values (as well as env vars) are all strings for the system, so we check "are all characters numbers? then this must be an integer, or a double if it has floating point" or "is the string true or false? then it must be a bool", etc. So if one looks like an integer and the other like a string, then this behavior would happen, unfortunately.

Regardless, I think this problem is an important one and it shouldn't be a limitation and shouldn't require you to change your env var values. I'm not sure if adding a way to force the type of a variable would be really desired (or good DX), so I'd imagine that ideally a solution would be to identify these scenarios and always fallback to both of them being the most common denominator (i.e. a String type).

Let me know the answer to my question above regarding the values of the env vars and I'll think about a solution for this 🙏

CarlosNano commented 3 months ago

Hi @rogerluan, thanks for the response.

Unfortunately both values in the .env contains only numbers.

The strange thing is for Debug it convert it to Int and for Release it convert it to String. When both are only numbers.

I just double checked that it has to do with the numbers because if I switch around keys then Debug converts to String and Release to Int.

Interestingly Release never conforms KeysEnvironmentProtocol because it's always expecting the same type than Debug

I wonder if the type inference is a solution a bit complex. Maybe an easy solution would be, an option in the arkana.yml file to not infer types and leave them always as a String?

rogerluan commented 3 months ago

an option in the arkana.yml file to not infer types and leave them always as a String?

That would be viable, yup! A lot easier than suppressing the type of each individual variable 😅

The reason one ends up being String and the other Int is because of this edge case that I had to add to prevent crashes:

https://github.com/rogerluan/arkana/blob/34826d6ae943980c79a3226947753312b731c7ef/lib/arkana/models/type.rb#L13-L18

I think your solution will end up being the chosen solution for this problem 😅

I'm in a hectic week this week and the next, would you like to take a stab at implementing this change and open a PR? Otherwise I'm afraid it might take some time before I can make this change 🙇

CarlosNano commented 3 months ago

Sure, I can try to give a go this weekend. Any advise where to start looking (in general terms) ?

rogerluan commented 3 months ago

Sorry @CarlosNano only seeing this now! Looks like this was already addressed here https://github.com/rogerluan/arkana/pull/72 — is that an alternate account of yours? 😁

EDIT: Nvm, just saw your comment in the PR desc! 🤗