Skip to content

Commit

Permalink
fix(types): widen too-narrow types (#102)
Browse files Browse the repository at this point in the history
* fix(types): widen too-narrow types

Since many functions accept `number`, which is then coerced to `string`, we need to account for that.

`Record<string, string>` for the type of `T` _would_ be fine, except:

1. `ProcessEnv` is `Record<string, undefined>`.  I'm guessing since you can write `process.env.cows = undefined`.  However, everything coming out of the actual environment in `process.env` is a `string`.
2. An empty object literal is not a `Record`, but it is an `object`. So we have to loosen the type arg `T` to extend `object`.

* chore(test): add types to tests and add a second fixture

- Renamed the existing fixture to `ts-project-module-nodenext`
- Added a new fixture `ts-project-module-commonjs` which is identical except it uses the `commonjs` `module` option and thus `node` `moduleResolution`
  • Loading branch information
boneskull authored Jan 10, 2024
1 parent 9549cc6 commit 6ad2e51
Show file tree
Hide file tree
Showing 17 changed files with 6,499 additions and 1,062 deletions.
14 changes: 14 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
"@eslint/js": "^8.49.0",
"@rollup/plugin-terser": "0.4.4",
"@tsconfig/node16": "^16.1.1",
"@types/chai": "^4.3.9",
"@types/mocha": "^10.0.3",
"@types/node": "20.8.9",
"chai": "4.3.10",
"eslint": "^8.21.0",
Expand Down
23 changes: 12 additions & 11 deletions src/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class EnvEmptyStringError extends Error {
/**
* A utility for interacting with environment variables
*
* @template {Record<string, string>} [T=Record<string, string>]
* @template {object} [T=Record<string, string|undefined>]
*/
export class Env {

Expand All @@ -118,7 +118,7 @@ export class Env {
*
* @overload
* @param {string} key The environment variable name to retrieve.
* @param {string} defaultValue The default value to return if the
* @param {string|number|boolean} defaultValue The default value to return if the
* environment variable is not found.
* @returns {string} The environment variable value if found or undefined if not.
*/
Expand All @@ -131,15 +131,16 @@ export class Env {
*/
/**
* @param {string} key
* @param {string} [defaultValue]
* @param {string|number|boolean} [defaultValue]
*/
get(key, defaultValue) {
let value = undefined;
if (typeof defaultValue !== "undefined") {
// this is defensive, since `defaultValue` is expected to be a `string`
value = String(defaultValue);
}

return (key in this.source) ? this.source[key] : value;
return (key in this.source) ? this.source[/** @type {keyof T} */(key)] : value;
}

/**
Expand All @@ -162,8 +163,8 @@ export class Env {
*
* @overload
* @param {string[]} keys The environment variable name to retrieve.
* @param {string} defaultValue The default value to return if the
* environment variable is not found.
* @param {string|number|boolean} defaultValue The default value to return if the
* environment variable is not found. Will be coerced to a string.
* @returns {string} The environment variable value if found or undefined if not.
* @throws {TypeError} If keys is not an array with at least one item.
*/
Expand All @@ -178,7 +179,7 @@ export class Env {
*/
/**
* @param {string[]} keys
* @param {string} [defaultValue]
* @param {string|number|boolean} [defaultValue]
*/
first(keys, defaultValue) {

Expand All @@ -188,7 +189,7 @@ export class Env {

for (const key of keys) {
if (key in this.source) {
return this.source[key];
return this.source[/** @type {keyof T} */(key)];
}
}
/** @type {string|undefined} */
Expand Down Expand Up @@ -252,7 +253,7 @@ export class Env {
get(target, key) {
key = String(key);
if (key in target) {
return target[key];
return target[/** @type {keyof T} */(key)];
}

throw new Env.KeyNotFoundError(key);
Expand Down Expand Up @@ -282,11 +283,11 @@ export class Env {
get(target, key) {
key = String(key);
if (key in target) {
if (target[key] === "") {
if (target[/** @type {keyof T} */(key)] === "") {
throw new Env.EmptyStringError(key);
}

return target[key];
return target[/** @type {keyof T} */(key)];
}

throw new Env.KeyNotFoundError(key);
Expand Down
17 changes: 8 additions & 9 deletions tests/env.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ describe("Env", () => {
const env = new Env(source);

assert.throws(() => {
// @ts-expect-error - bad type
env.first("USERNAME");
}, /First argument/);

Expand Down Expand Up @@ -182,6 +183,7 @@ describe("Env", () => {
const env = new Env(source);

assert.throws(() => {
// @ts-expect-error - 'USERNAME' not in source
env.requireFirst("USERNAME");
}, /First argument/);

Expand Down Expand Up @@ -254,6 +256,7 @@ describe("Env", () => {
const env = new Env(source);

assert.throws(() => {
// @ts-expect-error doesn't exist
env.exists.PASSWORD;
}, Env.KeyNotFoundError, /PASSWORD/);
});
Expand All @@ -277,18 +280,10 @@ describe("Env", () => {
const env = new Env(source);

assert.throws(() => {
// @ts-expect-error - intentional error
env.required.PASSWORD;
}, Env.KeyNotFoundError, /PASSWORD/);
});

it("should throw an error when the environment variable is an empty string", () => {
const env = new Env(source);

assert.throws(() => {
env.required.OTHER;
}, Env.EmptyStringError, /OTHER/);
});

});

describe("Custom Errors", () => {
Expand All @@ -299,12 +294,14 @@ describe("Env", () => {
};

class MyError1 extends Error {
/** @param {string} key */
constructor(key) {
super(key);
}
}

class MyError2 extends Error {
/** @param {string} key */
constructor(key) {
super(key);
}
Expand Down Expand Up @@ -349,6 +346,7 @@ describe("Env", () => {
const env = new Env(source);

assert.throws(() => {
// @ts-expect-error doesn't exist
env.exists.PASSWORD;
}, MyError1, /PASSWORD/);
});
Expand All @@ -357,6 +355,7 @@ describe("Env", () => {
const env = new Env(source);

assert.throws(() => {
// @ts-expect-error doesn't exist
env.required.PASSWORD;
}, MyError1, /PASSWORD/);
});
Expand Down
5 changes: 5 additions & 0 deletions tests/fixtures/ts-project-module-commonjs/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var env_1 = require("@humanwhocodes/env");
var env = new env_1.Env();
var username = env.get("USERNAME");
File renamed without changes.
Loading

0 comments on commit 6ad2e51

Please sign in to comment.