Incompatible sparqljs Type Definitions

[thatcort]

Describe the bug
This library includes its own type definitions for sparqljs. These definitions conflict with the ones hosted on DefinitelyTyped that are used by other projects. These incompatibilities make it very difficult to use sparql-engine in an application that also uses other libraries that depend on the DefinitelyTyped defintions. Importing from 'sparqljs' will resolve to @types/sparqljs rather than the custom ones (which is what is desired some of the time). Could you please transition to using the types provided by DefinitelyTyped (ideally from v3)?

To Reproduce
Steps to reproduce the behavior:

1. Create a TypeScript NodeJS application (see e.g. https://www.digitalocean.com/community/tutorials/setting-up-a-node-project-with-typescript)
2. Install sparql-engine and @types/sparqljs (or another library that depends on @types/sparqljs)
3. Attempt to create a custom sparql-engine Graph and import needed types from sparqljs
4. The system tries to import from @types/sparqljs instead of sparql-engine/types/sparqljs

I've managed to alter my .tsconfig file to prefer one set of definitions over the other, but this fails when it needs to use both.

Expected behavior
Only a single set of types should exist with the given name. Apps within the TypeScript SPARQL ecosystem should be able to share type definitions as much as possible.

The DefinitelyTyped definitions are here: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/sparqljs/index.d.ts

[Callidon]

Hi,

I'm very well aware of this issue. To give some context on this, we developed sparql-engine before the creation of the DefinitelyTyped definitions for sparql.js. However, before uploading types to DefinitelyTyped, the guys behind sparql.js made a major release that changed the whole API.

If we want to align the two type definitions, we will need to edit almost 80% of sparql-engine to modify types, because there is a strong coupling between the two packages. Indeed, we heavily use basic RDF types for sparql.js all over the code.

Of course, it's a major issue, and it should be fixed as soon as possible. However, I have not any free time to allocate to it before the end of the year. So, unless somebody is kind enough to contribute to this, we are stuck in the current situation.

[JuniperChicago]

Some time ago I did this kind of update on my local fork, though on an older version of sparql-engine and a version of sparql.js preceding the major API change. In the process of aforementioned work, I precipitated some updates to sparql.js itself, so I am familiar with the sections of code where these new conflicts can occur. I would be willing, to work on this update if nobody else is already taking it on. I have been away working on some other projects for a while but am finally back to working on some more RDF stuff.

[Callidon]

Just thinking about it, but a temporary solution could be to release on npm our own set of TypeScript definition for sparqljs@2.0.3, so you can use it in your project if you are also using version 2.0.3 of sparqljs. This is very easy to deploy on my side if you agree with it.

Of course, the long-term solution is to align ourselves with definitions available at DefinitivelyTyped, but this could be a good compromise in the meantime.

What do you think about it @thatcort ?

[thatcort]

I think it depends on when @JuniperChicago thinks he can complete the type migration. If it's soon, I'm managing to avoid the problem for now by working on other parts of my codebase and hoping it all gets resolved before I need to return to it. If he thinks it will be some time, then your workaround seems worthwhile. Thanks!

[JuniperChicago]

@thatcort @Callidon I am in the thick of it right now, and would like to be done this week. Is this fast enough? I am working through this task progressively, As an intermediate step, I am actually using the original /types/sparqljs/index.d.ts file, with its sparqljs module declaration and Algebra namespace and importing updated definitions through it. This allows some flexibility in the near term. This intermediate version I am working on sounds like what you are proposing @Callidon as a separate NPM import. So the question is, do you feel you can do this within a couple days with ease? If so, you might be moving a little faster than I am.

[thatcort]

That's fine for me. I have other deadlines to keep me busy until at least the end of the month. Plus I really appreciate the free labor that appeared as soon as I posted the issue, so whatever you can manage is amazing.

[Callidon]

Okay, fine for me too. I will upload to npm the "legacy" types package for sparqljs by the end of the day, and update sparql-engine as well. I will keep you updated.

[Callidon]

A (very late) update on this topic: I will publish the package and update sparql-js to use it today.

I apologize for the delay, but with the new lockdown in France happening at the same time as my Ph.D. defense, I've been very busy the past two weeks.

[Callidon]

And voilà, the new "legacy" types package is available as sparqljs-legacy-type. The README contains all instructions for its usage.

I've also updated sparql-engine to use this new package, instead of the types/sparqljs/index.d.ts file. I've also released a new version (v0.8.0) on npm with these changes.

@thatcort Do you think that's enough for your needs?

[thatcort]

Yes, seems so. Thank you!

[thatcort]

I tried using the updated library and unfortunately I don't think this approach will work. Because sparqljs-legacy-type is defining types for sparqljs, when trying to resolve sparqljs imports within my project files the types conflict. So configuring the tsconfig to use the correct definitions for sparql-engine will cause the types to not match what's expected by the other libraries and vice versa. Or maybe I'm misunderstanding something?

[thatcort]

Also congrats on your Ph.D. defense!

[Callidon]

Thanks 😄

The issue is that you cannot use both versions of SPARQL.js, because TypeScript can only compile using a single type definition file. That's a hard limitation of TypeScript, and I don't think it will ever change. If you use sparql-engine, then you must stick to version 2.0.3 of SPARQL.js, because that's the one used by our framework.

[JuniperChicago]

@Callidon @thatcort I am still working on contribution I commenced but had to pause for a week. It was my goal to use the latest sparql.js version where the first and most pervasive conflicts in type definitions center around the Algebra.TripleObject class which uses strings for each property. The newest version of sparql.js uses a spec compatible Quad. What slowed me down particularly was exploring if it would be easier, and without consequence to speed, to convert incoming Quad class objects to Algebra.TripleObject classes and vice versa, but that would require this conversion to occur at each modular interface or class extension. The other option is changing the classes and supporting functions throughout sparql-engine to allow the spec-compatible RDF.Quad objects to pass up to the point where bindings handled. The latter option would make module support easier, graph operations on spec-compliant modules easier too, and finally make communication modules used for federation more ubiquitous. I believe the best option for future usability is making the RDF.Quad a first class citizen of the engine. But it is a lot of work, which I am still committed to support.

[JuniperChicago]

@Callidon @Callidon What is your feeling about using the RDF.Quad to replace the Algebra.TripleObject? Because that is the path I am on right now.

[Callidon]

For me, the way to go is to replace all usage of Algebra.TripleObject by RDF.Quad, and have the whole API use dedicated RDF types for URIs, literals, and variables instead of strings. The other solution would be too counterintuitive and possibly introduce a huge technical debt. It was my plan to do this migration at the beginning of the year, but due to many "incidents" since then, I was not able to even begin the work.

@JuniperChicago If you are able to perform this migration, it could be a huge step forward for the framework 👍 Of course, the main drawback is that we introduce a major breaking change for all current users of the framework, as the API of all basic functions will drastically change. But I think such change is mandatory, and if we want to keep sparql-engine up to date, we will need to do it nonetheless.

[JuniperChicago]

@Callidon Great! I agree, and this approach appears consistent with the needs described by @thatcort, so will continue the path I am on.