olivernn / lunr.js

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

Improve the check for serialized index version #349

Open crystalfp opened 6 years ago

crystalfp commented 6 years ago

In lunr.Index.load() the version of the loaded index elicits only a warning if the two versions are different. Instead I think lunr.Index.load() should throw an error if the major version differs, output a warning if the minor version differs and display nothing if only the patch level differs (Semantic Versioning docet).

Just to be prepared for the future I added the following lines in my application as a workaround:

        // The index data is read in the data variable
    const serializedIndex = JSON.parse(data);
    const fileVersion = serializedIndex.version.split(".");
    const lunrVersion = lunr.version.split(".");
    if(fileVersion[0] != lunrVersion[0]) throw Error("Saved index major version mismatch");

    lunr.multiLanguage('en', 'it');
    fullTextIndex = lunr.Index.load(serializedIndex);
olivernn commented 6 years ago

The version check is very basic.

I want the schema for the serialised index to be defined outside of Lunr. This is so that the index can be generated without Lunr and a JavaScript runtime.

I'm mostly happy with the current schema, I just need to decide on how to version it, semantic versioning doesn't really make sense for a data format. Once I do that the current version field will be deprecated. At that point if the schema version does not exactly match what the version of lunr understands it should just fail hard.

crystalfp commented 6 years ago

OK, understand.

So do you think it is a "future oriented" move to wrap the Index.load() in a try/catch block and regenerate the index (instead of loading it) if the load fails? Thanks!

olivernn commented 6 years ago

When there is a real schema version, I would imagine that Lunr will just throw if there is a mismatch. You can certainly catch that and re-generate the index. I do not know what the specific error will be yet though.