fix: strengthen typings

[Lioness100]

An aspect of type safety is lost from this library because the get function isn't required to be a key of the container. For example:

const env = from({ foo: 'bar' });
env.get('food'); // typo! but typescript doesn't realize

With process.env, it is common for typescript users to augment the ProcessEnv:

declare global {
  namespace NodeJS {
    interface ProcessEnv {
      SOME_API_KEY: string;
    }
  }
}

process.env.SOME_API_KEY; // typo proof!

However, this isn't currently producible with this library. To fix this, I edited the types to add a new Container generic in IEnv to be used in the get() function.

const env = from({ num: 30 });

// before pr
env.get('number'); // undefined (typo)
env.get(); // { [varName: string]: string } (this isn't true!!)

// after pr
env.get('number'); // Type '"number"' is not assignable to type '"num"'. (2322)
env.get(); // { num: 30 } (accurate!!)

I also removed the PresentVariable generic because it... wasn't actually used in the interface.

[Lioness100]

A typescript bump to 3.9 is needed to use the @ts-expect-error directive.

[evanshortiss]

Great PR @Lioness100, will review shortly. I noticed that PresentVariable wasn't used either in the PR I opened, but didn't address it so thank you! 🤦

Also, apologies for the CI not working. I'll move from Travis to GH Actions soon since Travis seems completely broken for this specific repo.

[Lioness100]

Thanks!

[evanshortiss]

Published in 7.1.0! Thanks @Lioness100

[Lioness100]

Awesome!

[stuft2]

I thought the whole point of this library was to check if the variable existed in the "container" via the required() function and then to check the type via the other accessors. This was a bad change in my mind. Consumers of this package shouldn't have to specify the type of the container. They are asserting the type of the container using this package.

[Lioness100]

I'm not sure I understand your concern. My understanding is this package is not for asserting the type of the container, but for asserting the types of the containers properties. Furthermore, the typings change wasn't made so that the values could be inferred- you're right, that's what the existing functions are for. Instead, the typings are to assert that the names of the key inputted in get exists.

This doesn't actually change anything for most people. The ProcessEnv interface includes a mapped type [key: string]: unknown so that any property inputted would be valid.

Also, if you don't like asserting the Container very specifically... Just use Record<PropertyKey, unknown>. This will encompass any object.

[stuft2]

@Lioness100 It seems redundant to specify what properties I can call get() on since the package provides a way to assert that the properties exist via the required() function. Anyway, you're right that it doesn't change anything for most people. I was just surprise to find my code broken over a minor update. Most people probably aren't dependent on the Container type like my code is.

[Lioness100]

As shown in the original PR message - typos! If you want to get a variable API_KEY but accidentally type API)KEY, then the package can catch it for you. Imo the purpose of this package is to validate that properties exist on the code runner's side, not the code writer's side.

If your code breaks by this change, you are given an opportunity to strengthen your types 🙂 isn't that the whole point of typescript? And again, if you don't want to strengthen your types, you can use Record<PropertyKey, unknown>. Heck, even any would work!

[stuft2]

I understand the use case. It's still redundant since you can always do something like env.get('food').required() which will throw an error since food probably isn't going to be defined in process.env.

[Lioness100]

I'm not sure what you mean by that