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

feat: add more strict function definitions #35

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

garusis
Copy link
Contributor

@garusis garusis commented Oct 17, 2023

Currently addQueryToURL returns a union type forcing to always do some kind of type narrowing to the answer, even when is not necessary taking into account that in runtime the output will always have the same type of the input.

Before:

function onlyAcceptUrl(input: URL){...}
function onlyAcceptString(input: string){...}

const result = addQueryToURL(new URL('https://example.com/api?id=1'), {
     page: '2',
})
onlyAcceptURL(result instanceof URL ? result : new URL(result))
// OR
onlyAcceptURL(result as URL)

const result2 = addQueryToURL('https://example.com/api?id=1', {
     page: '2',
})
onlyAcceptURL(typeof result2 === "string" ? result : result.toString())
// OR
onlyAcceptURL(result2 as string)

Now:

function onlyAcceptUrl(input: URL){...}
function onlyAcceptString(input: string){...}

const result = addQueryToURL(new URL('https://example.com/api?id=1'), {
     page: '2',
})
onlyAcceptURL(result)

const result2 = addQueryToURL('https://example.com/api?id=1', {
     page: '2',
})
onlyAcceptURL(result2)

@gustavoguichard
Copy link
Owner

Hey @garusis ! I love your contribution and in fact I already had a branch to do the same: main...fix-addquerytourl-return-type
I prefer to use the generic approach instead of function overload although I'd love to have you as a contributor of this project.
Would you mind changing it to generic and then we can publish it?

@gustavoguichard
Copy link
Owner

@all-contributors please add @garusis for code, and bug.

@allcontributors
Copy link
Contributor

@gustavoguichard

I've put up a pull request to add @garusis! 🎉

@gustavoguichard gustavoguichard merged commit 2b58825 into gustavoguichard:main Oct 17, 2023
@garusis garusis deleted the add-generics branch October 17, 2023 20:30
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