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

Add TypeScript Defintion #58

Open
fuubi opened this issue Mar 5, 2020 · 4 comments
Open

Add TypeScript Defintion #58

fuubi opened this issue Mar 5, 2020 · 4 comments
Assignees
Labels
feature A new feature to be added

Comments

@fuubi
Copy link

fuubi commented Mar 5, 2020

I started to create the type definition files for @solid/query-ldflex, ldflex-comunica and ldflex on a forked DefinitelyTyped repository.

https://github.com/FUUbi/DefinitelyTyped/tree/master/types/ldflex
https://github.com/FUUbi/DefinitelyTyped/tree/master/types/ldflex-comunica
https://github.com/FUUbi/DefinitelyTyped/tree/master/types/solid__query-ldflex/

I did not properly test the definitions, it is more the result of my working progress.
There are some types, which I was not too sure about what they return... I simply defined "any" as a type for those instances.

Would it be possible that you look over it? That would be great. And further are you ok if it would be published later on, by sending a pull request to the original DefinitlyTyped repository?

@RubenVerborgh
Copy link
Member

Hi @fuubi,

Thanks, I do think that TypeScript bindings would be good to have.

There are a couple of reasons I didn't write LDflex in TypeScript, a major one being that we are doing really weird things with objects because of Proxy, so types would not help us as much as they would in other projects. Furthermore, the types used internally in LDflex are very different from those needed externally, again because of Proxy.

I have taken a look at the three typings above, https://github.com/FUUbi/DefinitelyTyped/blob/43ab08aa4e53e4e97e7a36b1845c824c96af3c55/types/ldflex/index.d.ts being the main one. While I really appreciate the effort, at the moment, they are not on the right track. The main concepts exposed by LDflex would be a Path and a ResolvedPath, where the latter is the result of awaiting the former. Furthermore, ResolvedPath would also implement RDF/JS Term.

For @solid/query-ldflex, we would likely need a generator to create the TypeScript definition, which would essentially compile the JSON-LD context.

@RubenVerborgh RubenVerborgh added the feature A new feature to be added label Mar 15, 2020
@fuubi
Copy link
Author

fuubi commented Mar 28, 2020

Hey Ruben, thank you very much for the appreciation.

I see. I went through the source code a couple of times, but never spotted that distinction. Unfortunately, the browser's console log only confronted me with a Proxy. Although I noticed the Proxy changed. I did not figure it out.

With your insights, I refactored the TypeDefinition. You can have a look at it here, hopefully, back on track. I am considering to remove the ability to get properties with a key { [ key : string ] : Path } since it resolves to the type any, which breaks type checking.

A TypeScript definition generator would defiantly be great. I had similar thoughts by defining the schema in a Turtle File. My initial plan was that the compilation output could look like this.

export type ProfilePath =  Path & {
    name : LiteralPath
    firiends: ProfilePath[] 
}

From an end-user perspective, I think I would be desirable to be able to get the friends name like this (await alice.friends).pop().name. But this will not work with a ProfilePath since we would have to call .toArray() first and then await it to be able to pop() a friend out of the array. A second problem, which I encountered, is that we can only retrieve properties represented within the source of the ComunicaEngine. Please, correct me if I am again missing something since it is not clear to me where it originates from. Is it a limitation inherited by the NSS or should the source of a ComunicaEngine be dynamically populated?

As an example:

const context = {
    '@context' : {
        '@vocab' :  'http://xmlns.com/foaf/0.1/' ,
        'friends' : 'knows'
    }
}

const queryEngine = new ComunicaEngine ( 'https://alice.solid.ma.parrillo.eu/profile/card' )
const path        = new PathFactory ( { context , queryEngine } )
const alice       = path.create ( { subject : namedNode ( 'https://alice.solid.ma.parrillo.eu/profile/card#me' ) } )

console.log ( ( await alice.name ).toString () )    
//  alice
console.log ( ( await alice.knows.toArray() ).pop().toString () )   
//  https://bob.solid.ma.parrillo.eu/profile/card#me
console.log ( ( await alice.knows.name ) )     
//  undefined 

The use of wrapper classes can solve both problems.
First, we need a base class Node, which appropriately defines the Path.

import data        from '@solid/query-ldflex'
import { Path }    from 'ldflex'

export abstract class Node {
    protected path : Path
    protected constructor ( path : Path ) {
        this.path = data[ path ]
    }

    getPath () : Path {
        return this.path
    }
}

Secondly, we create the Profile class, which derives from the Node class.

import { Node }              from 'domain/rdf/Node'
import {
    Path ,
    ResolvedPath
}                            from 'ldflex'
import { solid }             from 'rdf-namespaces'

export class Profile extends Node {
    constructor ( profile : Path ) {
        super ( profile )
    }

    async getName () : Promise<ResolvedPath> {
        return this.getPath ().name
    }

    async setName ( name : string ) : Promise<ResolvedPath> {
        return this.getPath ()
                   .name
                   .set ( name )
    }

    async friends () : Promise<Profile[]> {
        return this.getPath ()
                   .friend
                   .toArray ()
                   .then ( friends =>
                               friends.map ( f => new Profile ( f ) )
                   )
    }
}

Which allows us to use it as following:

const alice : Profile = new Profile ( 'https://alice.solid.ma.parrillo.eu/profile/card#me' )
const bob : Profile   = ( await alice.friends () ).pop ()

You can have a look at an implementation of mine, where I follow this architecture. It is really just first draft, so your feedback is welcome.

Would this "wrapper class" compile target be a possible option? I am also open to other ideas.

@RubenVerborgh
Copy link
Member

I went through the source code a couple of times, but never spotted that distinction.

Yeah, the LDflex source code is not a good estimate for what it will look like to consumers, given the use of Proxy.

With your insights, I refactored the TypeDefinition. You can have a look at it here, hopefully, back on track.

That is going in the right direction indeed, but still requires quite a bit of work for it to be usable to LDflex consumers.

For instance, termType in a resolved path would always be a string (Term['termType']), not a promise. And ResolvedPath would extend Path, not the other way round.

My initial plan was that the compilation output could look like this.

Yeah, that could be an option!

Secondly, we create the Profile class

I'd be careful with that; we might want to stay true to the notion of Linked Data, where there are no boundaries to a resource (whereas an object has boundaries).

I should probably write out the type in detail, given that most of this is still only in my head.

@fuubi
Copy link
Author

fuubi commented Apr 1, 2020

I'd be careful with that; we might want to stay true to the notion of Linked Data, where there are no boundaries to a resource (whereas an object has boundaries).

I would argue that if the goal is to optimise developer experience by getting type checking, we naturally introduce boundaries. Further, I think that most applications do add boundaries at their application layer. But I'm also okay with the other option. There we could define the ProfilePath type, which is well defined and bounded to a specific set of properties. Additionally, we provide a more general OpenPath type, which the former specific type can be cast to.

I should probably write out the type in detail, given that most of this is still only in my head.

That would be great. I am looking forward to seeing your thoughts be materialised. Until then, I'll give it some more thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature to be added
Projects
None yet
Development

No branches or pull requests

2 participants