Skip to content

Updates to indexing #334

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

Merged
merged 3 commits into from
Nov 19, 2019
Merged

Updates to indexing #334

merged 3 commits into from
Nov 19, 2019

Conversation

gkellogg
Copy link
Collaborator

@gkellogg gkellogg commented Nov 15, 2019

  • Indexing on @type requires @type to be either @id or @vocab, and defaults to @id.
  • Indexing on an arbitrary property, not just @index.

// If compactedItem contains a single entry
// whose key maps to @id, recompact without @type
if(Object.keys(compactedItem).length === 1 && '@id' in expandedItem) {
compactedItem = await api.compact({
Copy link
Member

@dlongley dlongley Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just get the alias for @id here and pull out the value compactedItem[alias] without having to run compact again? Or is it the case that it could have used a different value based on the presence of @type (via scoped contexts or something?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not easily, as if reduced to a node reference, it can be turned into a string. If not, it remains a node object. We could conceivably implement that logic there, but that's what expand is for, and it should be pretty quick in that case.

@dlongley
Copy link
Member

Modulo my comment this PR seems ok to me so far. @davidlehn ?

@gkellogg gkellogg marked this pull request as ready for review November 18, 2019 21:45
@davidlehn davidlehn added this to the JSON-LD 1.1 milestone Nov 19, 2019
@davidlehn davidlehn merged commit 6cbb389 into master Nov 19, 2019
@davidlehn davidlehn deleted the indexes branch November 19, 2019 18:50
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.

3 participants