krisk / Fuse

Lightweight fuzzy-search, in JavaScript
https://fusejs.io/
Apache License 2.0
18.36k stars 772 forks source link

fix & enhance type definition of search() #318

Closed ghost closed 5 years ago

ghost commented 5 years ago

TypeScript Playground Links: Current version / PR version


fuse.search should return string[] with option includes id

Current:

const fuse = new Fuse(list, { keys: ['title'], id: 'title' });
const res: string = fuse.search('keyword');
//err ^ Type 'Data' is not assignable to type 'string'.

PR:

const fuse = new Fuse(list, { keys: ['title'], id: 'title' });
const res: string[] = fuse.search('keyword'); // ok.

score: number | undefined is troublesome to handle with --strictNullChecks

Current:

const fuse = new Fuse(list, { keys: ['title'], includeScore: true });
const res = fuse.search('keyword');
console.log(res[0].score <= 0.5);
// err:     ^ Object is possibly 'undefined'.

PR:

const fuse = new Fuse(list, { keys: ['title'], includeScore: true });
const res = fuse.search('keyword');
console.log(res[0].score <= 0.5);  // ok.

with no-literal option object, TS cannot infer type of return of search() (#303)

Current:

const option: Fuse.FuseOptions<Data> = { id: 'title', keys: ['title'], includeScore: true };
const fuse = new Fuse(list, option);
const res0 = fuse.search('keyword')[0];
const id: string = res0.item;
// error:               ^ Property 'item' does not exist on type 'Data'.
const score: number = res0.score;
// error:                  ^ Property 'score' does not exist on type 'Data'.

this is not fixed, but now type inference can be overridden

PR:

const option:Fuse.FuseOptions<Data> = { id: 'title', keys: ['title'], includeScore: true };
const fuse = new Fuse(list, option);
const res0 = fuse.search<string, true, false>('keyword')[0];
const id: string = res0.item; // ok.
const score: number = res0.score; // ok.

248ee3176a9bf181d8425570981da82cedc4d695 is not working

Current:

const options: Fuse.FuseOptions<Data> = {
  id: 'author.lastName',
//^ Type '"author.lastName"' is not assignable to type '"title" | "author"'.
  keys: ['author.lastName']
//^ Type '"author.lastName"' is not assignable to type '"title" | "author"'.
}

PR:

const options: Fuse.FuseOptions<Data> = {
  id: 'author.lastName',  // ok.
  keys: ['author.lastName']  // ok.
}
falsyvalues commented 5 years ago

This looks far better than current one. I hope it will get released, thanks @konjac-potage!

krisk commented 5 years ago

Well done. I like it!

favna commented 4 years ago

It would be nice if the sample on the website can be updated to reflect these changes. Got a lot of build errors after upgrading but thankfully it was as easy as adding as V[] to my return statement (return fuzzyFuse.search(locquery) as V[];). I'm sure it won't be as easy a solution for everyone though.

alexkvak commented 4 years ago

Are you sure this is not breaking change? Or even minor release.