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

Remove generic parameters from DataFactory#fromTerm() overloads #57

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

Renegade334
Copy link
Contributor

Ref: #45

The current implementation of the overloads is a little confused – the generics here don't actually do anything, and having multiple generic overloads like this has the potential to cause assignability headaches with the TypeScript compiler.

This change simply removes those generics. The catch-all signature is unnecessary, as all cases are already covered.

Copy link

changeset-bot bot commented Jan 14, 2025

🦋 Changeset detected

Latest commit: 49b9476

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@rdfjs/types Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@tpluscode tpluscode requested a review from rubensworks January 14, 2025 07:55
@tpluscode
Copy link
Contributor

Thank you @Renegade334. I saw PR DefinitelyTyped/DefinitelyTyped#71645. If we merge this, will we need to do anything to DT? The idea os that v2 is actually backwards-compatible with v1

@tpluscode
Copy link
Contributor

@rubensworks maybe we overdid this in #45. The test do not fail so if you cannot think of any uncovered scenario, I say we merge this ASAP and push 2.0.1

Copy link
Member

@rubensworks rubensworks left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Tried this out in my repos, and this doesn't seem to be causing any breakages on my end.

@tpluscode tpluscode merged commit 6f54811 into rdfjs:master Jan 14, 2025
3 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.

3 participants