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

Compatibility with RDF/JS types #374

Open
joepio opened this issue Dec 10, 2019 · 6 comments
Open

Compatibility with RDF/JS types #374

joepio opened this issue Dec 10, 2019 · 6 comments

Comments

@joepio
Copy link
Collaborator

joepio commented Dec 10, 2019

The RDF/JS taskforce data model spec specifies a set of classes and methods for RDF related models.

During the migration to Typescript (#373 #355) I decided to use RDF/JS types, and extend these in RDFlib when necessary. However, another set of typescript types already exists. I tried to use these types in RDFlib, but I could not resolve the type errors.

The most fundamental problem is that @types/rdf-js defines Term as a Union of its other Terms:

export type Term = NamedNode | BlankNode | Literal | Variable | DefaultGraph;

The RDF/JS spec defines Term as an abstract interface.

Since RDFlib uses custom terms (like Collection), it is compatible with the RDF/JS spec, and it works with my types, but it does not work with the types from @types/rdf-js. This problem with Term, unfortunately, trickles down quite a bit and makes it impossible to use these types, at the moment.

There are a couple of things we can do here:

  1. Issue a PR for DefinitelyTyped/rdf-js, change Term to fit the spec. This does require that these changes do not break anything in rdf-js.
  2. Add the types to the RDF/JS repo. I think it would make sense if these types live in a separate repo, controlled by the RDF/JS taskforce.
  3. Use the tf-types.ts that is included in Typescript #373
@joepio joepio mentioned this issue Dec 10, 2019
@RubenVerborgh
Copy link
Member

Let’s already open discussion for 1. Perhaps Collection could be a subclass of BlankNode; that actually makes sense.

@joepio
Copy link
Collaborator Author

joepio commented Dec 10, 2019

Working on it. I've reached out to the editor of the types and I've created a fork for PR.

@thewilkybarkid
Copy link

In the spec and RDF/JS repo it's a little unclear where extension should be allowed. Despite the IDL in the spec using Term in Quad, the wording suggests that only specific types are meant ('... subject, which is a NamedNode, BlankNode or Variable' etc). This is split in the repo into BaseQuad and Quad, which makes it a little confusing (means that other types are allowed). Either way, subtypes are allowed (eg Collection being a BlankNode as mentioned), and the generics approach should allow you to create a new BaseQuad subclass if needed.

If subclassing Term directly isn't meant to be allowed, and using non-standard terms on a Quad directly is also not meant to be allowed, I think the repo could be simplified by removing BaseQuad.

If subclassing Term is meant to be allowed, then it should be changed to the interface you mentioned. (I think Quad_Subject etc should also become interfaces too.)

@tpluscode
Copy link

The current types are strictly related to the core building blocks of RDF. The BaseQuad/Quad split is a little confusing but actually useful because not every kind of term is allowed on every position in a quad (for example blank node cannot be a predicate).

I have not looked at the code being discussed but it is definitely possible to extend a term. For example you might define a Collection interface to extend a BlankNode.

const factory: DataFactory = <any> {};

interface Collection extends BlankNode {}

const collectionNode: Collection = <any> {};
const predicate: NamedNode = <any> {};
const obj: NamedNode = <any> {};

factory.quad(collectionNode, predicate, obj);

@RubenVerborgh
Copy link
Member

CC: @rubensworks

@rubensworks
Copy link

I'm fine with making Terms more extensible in the typings, but we have to take into account that this would break certain TS implementations that depend on these strict types.

For reference, this was the reasoning for making the typings like this a couple of years ago: https://lists.w3.org/Archives/Public/public-rdfjs/2017Sep/0001.html

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

No branches or pull requests

5 participants