olivernn / lunr.js

A bit like Solr, but much smaller and not as bright
http://lunrjs.com
MIT License
8.91k stars 546 forks source link

Result metadata for non-initial search token is nested under term instead of field. #320

Closed jgerigmeyer closed 6 years ago

jgerigmeyer commented 6 years ago

Given a multiple-word search token (e.g. foo bar), the result position metadata is improperly nested under a key matching the search term (bar) instead of the index field (e.g. title).

For example, given the following data and search:

var idx = lunr(function() {
  this.ref('name');
  this.field('text');
  this.field('title');
  this.metadataWhitelist = ['position'];

  this.add({
    "name": "name",
    "title": "foo bar",
    "text": "foo bar buz baz",
  });
});
var result = idx.search('baz foo buz');

The expected result matchData.metadata would be:

{
  "baz": {
    "text": {
      "position": [[12,3]]
    }
  },
  "foo": {
    "text": {
      "position": [[0,3]]
    },
    "title": {
      "position": [[0,3]]
    }
  },
  "buz": {
    "text": {
      "position": [[8,3]]
    }
  }
}

However, the actual result (using Lunr.js v2.1.4) is:

{
  "baz": {
    "text": {
      "position": [[12,3]]
    }
  },
  "foo": {
    "foo": {
      "position": [[0,3]]
    },
    "title": {
      "position": [[0,3]]
    }
  },
  "buz": {
    "buz": {
      "position": [[8,3]]
    }
  }
}

See it live in a jsfiddle: https://jsfiddle.net/jgerigmeyer/3k21gft9/2/

olivernn commented 6 years ago

I think this is due to passing the wrong arguments to a method here - https://github.com/olivernn/lunr.js/blob/master/lib/index.js#L241

I actually noticed this the other day when working on adding AND query support. My initial thought was that this branch was dead code since the passed arguments looked obviously wrong and there had been no reports of odd behaviour. Looks like I was wrong!

I'll put together a patch to fix this and will get a release out in the next few days, thanks for reporting.

jgerigmeyer commented 6 years ago

@olivernn I went ahead and submitted a PR to fix this: #321.