N3.js
N3.js copied to clipboard
Outdated type information in @types/n3
As the title says, the type information in @types/n3
is outdated. There was only 1.10.4
, while the current version for n3
is 1.16.1
.
At least one incompatibility is the readQuads()
method for Store
. It exists in (at least) 1.16.0
, while the type information in 1.10.4
does not have it.
Not sure if I should report this here or in https://github.com/DefinitelyTyped/DefinitelyTyped. But as the maintainers here also contributed to that repo, I guess starting from here is the best option.
I'd be inclined to say that readQuads
should be reserved as an internal property and not exposed in the typings.
Do you have a use case for it that cannot be achieved with the match
method?
If it's not underscored, it was considered public. Any negatives to exposing it?
Mainly just to leave the option of changing its behaviour in the future - for instance in work on https://github.com/rdfjs/N3.js/issues/281 or https://github.com/comunica/comunica/issues/594#issuecomment-1099889243, though such issues may not require changes to its behaviour anyway
Do you have a use case for it that cannot be achieved with the match method?
@jeswr I don't know. It is listed first in the document, so I used it.
I could try changing it to match()
. But I guess this indicates problems with the document? (The document is not very easy to catch up with for me as a newbie, by the way.)
Fair enough - in general I would recommend selecting those methods defined in the DatasetCore definition https://github.com/rdfjs/types/blob/183bda795f57a9464ddf95deac45a0c4a48879cf/dataset.d.ts#L7 wherever possible as this means your app can stay compatible with other dataset implementations as well.
Indeed this may also be a point worth noting in the docs @RubenVerborgh ?
Mainly just to leave the option of changing its behaviour in the future
In light of the fact that it is mentioned in the documentation, this is a moot point anyway - I now agree that it should be added to the @types/n3 definition.
Hello, I was going to open a PR but the title of this one seems appropriate.
I was trying to parse an rdfStream.
The README says that you can (line 147):
const parser = new N3.Parser(),
rdfStream = fs.createReadStream('cartoons.ttl');
parser.parse(rdfStream, console.log);
However when I tried, Typescript complained:
typescript: Argument of type 'ReadStream' is not assignable to parameter of type 'string'
I checked the types and they also don't seem to allow ReadStream.
But then I checked the code and it seems to actually support streams.
So the types don't seem to match the code
Thanks!
@andrefs Indeed, feel free to PR.
What is needed to update the types? It kinda hard to use the lib in typescript right now; at least for a novice.
Another question I had: why is the type Quad_Object
used instead of Term
at https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/n3/index.d.ts#L82 ?
PR started in https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65275
@jeswr you were willing to review the PR above?
Sure @mielvds - is there a reason it is still in draft mode?
no, just me being insecure on what I'm doing :)