digitalbazaar / jsonld.js

A JSON-LD Processor and API implementation in JavaScript
https://json-ld.org/
Other
1.66k stars 195 forks source link

Minimal scoped context affects term selection with index container #394

Open gkellogg opened 4 years ago

gkellogg commented 4 years ago

From https://github.com/w3c/wot-thing-description/issues/894

Compacting the following:

{
  "https://www.w3.org/2019/wot/td#hasPropertyAffordance": [
    {
      "http://purl.org/dc/terms/title": "Switch"
    }
  ],
  "http://purl.org/dc/terms/title": "46203"
}

with the following context:

{
  "@context": {
    "td": "https://www.w3.org/2019/wot/td#",
    "dct": "http://purl.org/dc/terms/",
    "properties": {
      "@id": "td:hasPropertyAffordance",
      "@type": "@id",
      "@container": "@index",
      "@context": {}
    },
    "title": "dct:title"
  }
}

prevents "title" from being used in the output:

{
  "@context": {
    "td": "https://www.w3.org/2019/wot/td#",
    "dct": "http://purl.org/dc/terms/",
    "properties": {
      "@id": "td:hasPropertyAffordance",
      "@type": "@id",
      "@container": "@index",
      "@context": {}
    },
    "title": "dct:title"
  },
  "title": "46203",
  "properties": {
    "@none": {
      "dct:title": "Switch"
    }
  }
}

Instead, it uses "dct:title". Removing "@context": {} from the context makes it work and use "title" instead.

gkellogg commented 4 years ago

See https://github.com/w3c/wot-thing-description/issues/894 and playground.

davidlehn commented 4 years ago

Tests over here: https://github.com/w3c/json-ld-api/pull/490 It's failing at some low level with new basic tests. Haven't debugged yet, but looks like scoped contexts are just overwriting top level contexts?

davidlehn commented 4 years ago

Bisect for basic additive scoped context fails here:

davidlehn commented 4 years ago

In the code that does processing just to validate, I think the bug can be fixed with activeCtx: rval.clone() instead of just rval. That seems an unfortunate efficiency issue. Is the passed activeCtx supposed to be mutated?

gkellogg commented 4 years ago

There was code to freeze these contests, but it was too expensive too. Might be worth doing it to see where it’s being modified and only duo when necessary.

RoboPhred commented 4 years ago

Given the lack of activity in this issue, I implemented the fix mentioned above and tested it with the updated test suite.

Fix in MR #411

davidlehn commented 3 years ago

The proposed patch was added to jsonld.js 3.2.0 and is live on playground now. I think this issue is fixed. Please test. There is the possibility of a performance regression due to this, but another issue was opened to handle that case.