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

fix: handle template literals in startsWith #182

Merged

Conversation

jly36963
Copy link
Contributor

@jly36963 jly36963 commented Mar 14, 2024

Relates to: #181

Description

I needed to have more granular control over the comparison, so I replaced the
${S}${string}
part with
${infer THead}${infer TRest}

I needed to remove Length<T> in a lot of places -- it doesn't work with template literals.

Details

Length does not (and cannot) handle string interpolation well.

Length<'abc'> // 3
Length<`abc${string}`> // number

Slice<T, startIndex, endIndex> uses Length<T> as the default value for endIndex.
Both StartsWith/EndsWith use Slice.

I refactored Slice/StartsWith/EndsWith to not use Length (or only use it when T is known to be a literal) and I now have them working with template literals.

Also, slice/startsWith/endsWith are not methods with default number arguments (eg: 0 or Length<T>), they are overloads where the last arguments are optionally provided.

Consider

slice(start?: number | undefined, end?: number | undefined): string
startsWith(searchString: string, position?: number | undefined): boolean
endsWith(searchString: string, endPosition?: number | undefined): boolean

In other words, updating the params for these types to be
P: number | undefined = undefined
instead of
P: number = Length<T>
makes this fix possible and more closely aligns our types with the TS definitions for those string methods.

@jly36963 jly36963 marked this pull request as ready for review March 17, 2024 16:06
Copy link
Owner

@gustavoguichard gustavoguichard left a comment

Choose a reason for hiding this comment

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

All good to me! The code is more complex than what I wanted but I trust you took the right decisions!
TY very much for the PR!!

@gustavoguichard gustavoguichard merged commit dff8152 into gustavoguichard:main Mar 18, 2024
7 checks passed
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