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

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

Closed
wants to merge 1 commit into from

Conversation

tpluscode
Copy link
Contributor

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
Copy link
Contributor Author

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

@jeswr
Copy link
Collaborator

jeswr commented Mar 20, 2023

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

@tpluscode
Copy link
Contributor Author

Yes, all pass locally.

@jeswr
Copy link
Collaborator

jeswr commented Mar 21, 2023

@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
Copy link
Contributor Author

I'd suggest re-running the CI.

Can you do that?

@jeswr
Copy link
Collaborator

jeswr commented Mar 22, 2023

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

@RubenVerborgh
Copy link
Member

@jeswr Did both 👍

@jeswr jeswr changed the base branch from main to versions/2.0.0 March 27, 2023 05:01
@jeswr jeswr mentioned this pull request Mar 27, 2023
@jeswr
Copy link
Collaborator

jeswr commented Mar 27, 2023

Closing in favour of #346

@jeswr jeswr closed this Mar 27, 2023
@tpluscode
Copy link
Contributor Author

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

@jeswr
Copy link
Collaborator

jeswr commented Mar 28, 2023

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 jeswr reopened this Mar 28, 2023
@jeswr jeswr changed the base branch from versions/2.0.0 to main March 28, 2023 23:10
@jeswr jeswr mentioned this pull request Mar 28, 2023
@jeswr jeswr closed this Mar 28, 2023
@jeswr
Copy link
Collaborator

jeswr commented Mar 29, 2023

Fixed in 1.16.4

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

Successfully merging this pull request may close these issues.

Cannot use alternative factories in n3 mode
3 participants