src/collection.ts: Incompatible method override of `substitute()`

[moritzsommer]

Current Situation

The substitute() method of the class Collection returns a new Collection<Node | Collection<any> | Literal | Variable>:

rdflib.js/src/collection.ts

Lines 104 to 108 in 46302d5

	substitute (bindings: Bindings) {
	const elementsCopy = this.elements.map((ea) => ea.substitute(bindings))
	return new Collection(elementsCopy) as Collection<Node | Collection<any> | Literal | Variable>
	}

This creates an incompatible method override with the corresponding substitute() method of the superclass Node, which returns the local type parameter T:

rdflib.js/src/node-internal.ts

Lines 35 to 37 in 46302d5

	substitute <T extends Node = Node>(bindings: Bindings): T {
	return this as unknown as T
	}

The superclass method allows the caller to choose any T extends Node, while the subclass method always returns a specific Collection<...>. Therefore, Collection no longer fulfils the method contract of Node.

As a result, compiling the package declarations produces a TS(2416):

node_modules/rdflib/lib/collection.d.ts:52:2 - error TS2416:
Property 'substitute' in type 'Collection<T>' is not assignable to the same property in base type 'Node'

Reproduction

Install the package via npm install rdflib@2.4.0. A minimal reproducible example repro.ts, compiled via npx tsc repro.ts --noEmit, would be:

import Collection from "rdflib/lib/collection";
import Node from "rdflib/lib/node-internal";

const node: Node = new Collection();
console.log(node)

This produces the TS(2416) together with all resulting subsequent type errors.

Proposed Change

Replace the return type of the superclass method substitute():

substitute(bindings: Bindings): Node {
	return this;
}

Subclasses could then safely override the method with a more specific covariant return type like Collection<...>.

Moreover, I am wondering why this has not caused problems before, but I cannot see an application error in the reproduction above either. Did I miss something?

I am also not sure how external contributions are handled in this repository. I would really like to open a small PR for this change if you agree, and add a consumer type check to the CI so that the generated declaration files are type-checked as well.

[bourgeoa]

Thanks. PR are welcomed. Review may take some time.
Easier with tests.