Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: upgrade Typescript to 5.7.2 and @types/node to 20.12.2 #104

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonahtylerb
Copy link

No description provided.

@jonahtylerb jonahtylerb force-pushed the jbrupbacher/upgrade-typescript branch 2 times, most recently from 9627c53 to 0e7e438 Compare December 31, 2024 15:57
@jonahtylerb jonahtylerb changed the title chore: upgrade Typescript to 5.7.2 chore: upgrade Typescript to 5.7.2 and @types/node to 20.12.2 Dec 31, 2024
@@ -1,4 +1,4 @@
export function delay<T = undefined>(ms: number, value?: T): Promise<T> {
export function delay(ms: number, value?: undefined): Promise<undefined> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the generic will break usages like:

const val: string = await delay(100, 'cat');

Does something like this work? It keeps the internal API agnostic of the type of value but external consumers have the option to return a value and have the proper return type inferred.

Suggested change
export function delay(ms: number, value?: undefined): Promise<undefined> {
export function delay(ms: number): Promise<undefined>;
export function delay<T>(ms: number, value: T): Promise<T>;
export function delay<T>(ms: number, value?: T): Promise<T | undefined> {

@@ -2,7 +2,7 @@
* Type guard to check to see if the given value is a valid value for the enum.
*/
export function isEnumValue<T>(enumType: T, value: unknown): value is T[keyof T] {
return (Object.keys(enumType) as Array<keyof T>)
return (Object.keys(enumType as {}) as Array<keyof T>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as casts can be dangerous. As such, we avoid them if at all possible. Would adjusting the function signature to the following example remove the need for the cast?

export function isEnumValue<T extends object>(enumType: T, value: unknown): value is T[keyof T] {

@@ -4,7 +4,7 @@ import { delay, isPromiseLike } from '../../src';
describe('delay', () => {

it('delays before returning a value', async () => {
let after: string | null = null,
let after: string | undefined | null = null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the types for delay change, this may no longer be needed.

@@ -31,7 +31,7 @@ describe('isEmpty', () => {
expect(t.isEmpty(() => {})).to.strictlyEqual(true);

// deleting all the keys from on object empties it
o = { a: true };
o = { a: true } as { a?: boolean };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same note about as casting.

Suggested change
o = { a: true } as { a?: boolean };
const o: { a?: boolean } = { a: true };

@jonahtylerb jonahtylerb force-pushed the jbrupbacher/upgrade-typescript branch from 0e7e438 to 276b464 Compare January 1, 2025 13:37
@jonahtylerb jonahtylerb force-pushed the jbrupbacher/upgrade-typescript branch from 276b464 to 8b9058a Compare January 1, 2025 21:31
@jonahtylerb jonahtylerb force-pushed the jbrupbacher/upgrade-typescript branch from 8b9058a to fec0370 Compare February 14, 2025 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants