rdfjs / N3.js

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

`removeMatches` removes quads from `Store` asynchronously #373

Closed mrkvon closed 7 months ago

mrkvon commented 7 months ago

When we remove quads from Store with removeMatches, they seem to stay in the Store until next tick. I'd expect them to be removed immediately. Also seems to be the case with deleteGraph.

You can see it reproduced in this code sandbox.

Example code

(async () => {
  const store = new n3.Store(); // new empty store

  console.log("number of quads in store:");

  console.log(store.size, "before addition"); // we expect 0 here, get 0

  store.add(
    new n3.Quad(
      new n3.NamedNode("https://subject"),
      new n3.NamedNode("https://predicate"),
      new n3.NamedNode("https://object"),
    ),
  );

  console.log(store.size, "after addition"); // we expect 1 here, get 1

  store.removeMatches(); // remove everything

  console.log(store.size, "synchronously after removal"); // we expect 0 here, get 1 <=== This is the problem

  await new Promise(process.nextTick); // wait for the next tick

  console.log(store.size, "next tick after removal"); // we expect 0 here, get 0
})();
jeswr commented 7 months ago

Note that this library implements removeMatches as an event emitter; in particular it is implemented as follows https://github.com/rdfjs/N3.js/blob/d3faa57c82c3c46a667bc3e31312f7b5d82ff8da/src/N3Store.js#L357-L367.

So I would not expect the matches to be removed until the end event of the returned event emitter has been called.

Are there docs / types somewhere that indicate this should be synchronous?

joachimvh commented 7 months ago

It's because N3.js implements removeMatches as described in the RDF/JS Stream spec here.

I think the only sync solution is to call getQuads and then removeQuads with the result.

mrkvon commented 7 months ago

I've seen this comment by @RubenVerborgh:

The N3.Store interface is synchronous [...]

Which i guess is slightly incorrect, then.

I hadn't noticed the links to the external specs, thanks @joachimvh for pointing them out, and for the workaround.

Also, thank you all for promptly resolving this. 🙂

RubenVerborgh commented 7 months ago

Back in 2018 it was correct 😅