feat: make TermWrapper generic over the wrapped term (TermWrapper<T extends Term = Term>)#85
Draft
jeswr wants to merge 1 commit into
Draft
feat: make TermWrapper generic over the wrapped term (TermWrapper<T extends Term = Term>)#85jeswr wants to merge 1 commit into
jeswr wants to merge 1 commit into
Conversation
Adds a class-level type parameter with a default, TermWrapper<T extends Term = Term>, so the type of the wrapped term is inferred from the constructor argument and termType narrows to T["termType"]. The default keeps every existing declaration and call site compiling unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 6, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes
TermWrappergeneric over the term it wraps:What it does
The
Termconstructor overload now takesTandoriginalis typedT, so the wrapped term type is inferred at the construction site:new TermWrapper(factory.literal("some value"), dataset, factory)is aTermWrapper<Literal>.termTypereturnsT["termType"], so it narrows to the literal type of the wrapped term ("Literal","NamedNode", …). The compiler catches impossible comparisons (literalWrapper.termType === "NamedNode"is a build error) and unions of wrappers discriminate on it:The parameter defaults to
Term, so the change is non-breaking: bareTermWrapperreferences,class SomeClass extends TermWrapper,ITermWrapperConstructorand all mapping helpers compile unchanged, andTermWrapper<Literal>remains assignable toTermWrapper(covariant). The string overload of the constructor is untouched (the IRI is wrapped as aNamedNode;Tstays at the default there).Why
This is the class-generic slice of the typing work in #61/#74, isolated so it can be discussed on its own merits. The rationale is the one @jeswr gave in #61 (comment): errors at build time instead of run time, less
asfor consumers, and avoiding false types — with the editor able to narrow what a wrapper can be based on the term it wraps. Because this slice keeps the default parameter and does not touch the getters, consumers who never name the parameter see exactly the API they see today.What it deliberately does not do
Literal/Quadconvenience getters (language,direction,datatype,subject, …) remain available on every wrapper regardless ofT. Conditioning those on the wrapped term type is the contested part of IntroduceTermWrapper#fromandTermWrapper#in#61 and is left out here.TermNode), noIRdfJsTermchanges, no#in.TermWrapper.from— that factory is proposed separately in feat: TermWrapper.from static factory with intersection return type #83 (whoseInstanceType<This> & Treturn type already carries the term type). This PR and feat: TermWrapper.from static factory with intersection return type #83 touch adjacent code and merge cleanly in either order; connecting the two is a trivial follow-up once both land.Relation to existing PRs
Supersedes the generic-parameter slice of #61 and of #74 (the
class TermWrapper<T extends Term>declaration itself). The remaining slices of those PRs (from/infactories → #83, getter/IRdfJsTermrestructuring,TermNodemapper signatures) are intentionally not included.Testing
test/unit/term_wrapper_generic.test.ts: runtime assertions plus compile-level assertions — inference from the constructor, discriminant narrowing of wrapper unions, covariant assignability to the defaultTermWrapper, subclasses with an explicit type argument working with the mapping helpers,@ts-expect-errorproving the impossible-comparison diagnostic, and the term-type-specific getters remaining available.npm test: 131 tests, 0 fail (5 pre-existing skips); the new constructor/getter paths are covered.npx typedoc: 0 errors, 6 warnings — identical tomain.