rdfjs / N3.js

Lightning fast, spec-compatible, streaming RDF for JavaScript
http://rdf.js.org/N3.js/
Other
676 stars 127 forks source link

fix: use BlankNode according to RDF/JS spec #335

Closed tpluscode closed 1 year ago

tpluscode commented 1 year ago

Fixes #334

When parsing n3 rule, the parse tries to access .id of a term which is not according to spec and fails when alternative factory is provided, such as @rdfjs/data-model. This PR's change is to the .value property instead

tpluscode commented 1 year ago

Uh, why did the tests fail? They work fine locally here

jeswr commented 1 year ago

Spec tests are not part of the unit tests, have you run npm run spec when working locally?

tpluscode commented 1 year ago

Yes, all pass locally.

jeswr commented 1 year ago

@tpluscode - it looks like the problem was that the mf:result http://w3c.github.io/rdf-tests/turtle/IRI_spo.nt wasn't downloaded properly by the test suite - because the error


✖ comment_following_localName
  comment following localName
  Error: Invalid data parsing
  Input: @prefix p: <http://a.example/> .
<http://a.example/s> <http://a.example/p> p:o#comment
.

  Expected: []

  Got: [
  {
    "subject": "http://a.example/s",
    "predicate": "http://a.example/p",
    "object": "http://a.example/o",
    "graph": ""
  }
]

incorrectly asserts that the result should be empty when the result in the file is indeed <http://a.example/s> <http://a.example/p> <http://a.example/o> .. I'd suggest re-running the CI.

tpluscode commented 1 year ago

I'd suggest re-running the CI.

Can you do that?

jeswr commented 1 year ago

@RubenVerborgh - can you re-run the CI on this or give me access to do so?

RubenVerborgh commented 1 year ago

@jeswr Did both 👍

jeswr commented 1 year ago

Closing in favour of #346

tpluscode commented 1 year ago

What is the timeline for v2? A simple one-liner could still make it to 1.16.x, surely? 🙏

jeswr commented 1 year ago

Aiming for this week; and the breaking changes should not require any changes in most consumers (it changes the semantics of parsing <= in N3 from log:implies to log:isImpliedBy in N3, enables RDF-star support by default, and doesn't expose term constructors that should have always been internal anyway).

Will make a patch release with this regardless.

jeswr commented 1 year ago

Fixed in 1.16.4