I've done some initial work to try to fix type errors that pop up when using the work on the master branch (e.g. for Solid UI it outputs 209 errors).
Doing this I came across a couple of issues that can be split into two groups of problems:
- Classes that do not extend their parents properly
- Difficulties in combining types that relate to RDFJS and RDFLIB.
Problem 1
For problem 1 I've had a talk with Tim, and he thinks some reasonable fixes are:
- To match the signature of
Formula#add
conform to IndexedFormula#add
- To rename
IndexedFormula#compareTerm
to IndexedFormula#compareTerms
We didn't go into depth of other inconsistencies, but it seems that we will have to break some APIs in any case if we want to fix this properly (currently we've used @ts-ignore
to tell TypeScript to ignore them - but this won't fix the problem for projects that depend on this classes, as TypeScript will complain about inconsistent types here as well, which leads to a lot of unneccessary uses of ts-ignore).
Problem 2
Problem 2 seems to come from the fact that many outputs from methods now uses the types described in the RDF/JS Data model specification (I'll just refer to them as RDF/JS types later in this text) instead of the corresponding rdflib ones, and this causes errors for projects such as Solid UI which has worked with rdflib types till now. This has been discussed earlier at https://github.com/linkeddata/rdflib.js/issues/374 when @joepio worked on this earlier (and an even older thread at https://github.com/linkeddata/rdflib.js/issues/137).
The data model types exported from rdflib (e.g. by doing import { NamedNode } from 'rdflib'
) still refer to rdflib types, which causes type errors for projects such as Solid UI (one fix is to change all type references to the corresponding RDF/JS types, e.g. by doing import { NamedNode } from 'rdflib/src/tf-types'
, but I think this is a very bad solution.
One simple fix is to make sure that the types that TypeScript refers to are in fact referring to the RDF/JS types. But this will make it more difficult for people to use the extra functionality that is available in the rdflib types.
Now, Tim wants to make sure that types returned from rdflib are using the rdflib types (e.g. so that developers have access to some extra methods, such as toNT
, doc
, and site
).
Since they are a superset, one theory of mine was to allow RDF/JS types as input for all relevant methods, but that they output rdflib types. The idea is that developers who want to work with RDF/JS can still do it (since rdflib types are a superset of RDF/JS types).
I started doing this work on a branch called fix-types-hybrid for those who want to review what I tried doing (note: it's incomplete code, so it won't run and there are bugs that I would've fixed before a proper PR).
When working on the branch I realized I was getting into a mess, and decided to raise these issues to get some feedback on how we might want to go about this.
Another challenge I found wrt types are unions that are different between RDF/JS and rdflib. E.g. for rdflib we have a type that describes which types we allow for many parameters that expect an object (as in subject-predicate-object):
type ObjectType = RDFlibNamedNode | RDFlibLiteral | Collection | RDFlibBlankNode | RDFlibVariable | Empty
While in RDF/JS it looks like the following:
type Quad_Object = NamedNode | BlankNode | Literal | Variable | Term
Please don't mind the different names, the important difference here are the types that don't match in the unions; rdflib allows for Collections
and Empty
, while RDF/JS allows for Term
. I think we might still get the concept to work (allow for RDF/JS types, output rdflib types), but I think it needs a bit of work.
I also tried to work on another branch where the idea was to remove the RDF/JS types altogether, but I realized it was going to require a lot of work and I didn't want to revert the work of @joepio and @fletcher91 before having a discussion on this. For those interested in reviewing that work, it's available on branch fix-types-rdflib.
Hope to get feedback on this from @timbl, @RubenVerborgh, perhaps @dmitrizagidulin and @elf-pavlik who has been involved in these discussions earlier, as well as @joepio and @fletcher91 in case I forgot something in this issue, and @Vinnl who helped me with my thinking on TypeScript types earlier.