flexsearch icon indicating copy to clipboard operation
flexsearch copied to clipboard

Incorrect type definitions

Open dixiekong opened this issue 3 years ago • 20 comments

The supplied type definitions are not correct importing the Index as such:

import { Index } from 'flexsearch';
const index = new Index();

results in the error: 'Index' only refers to a type, but is being used as a value here

There exits working type definitions in https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/flexsearch but to use them one has to either downgrade to a version containing no type definitions (e.g. 0.7.11) or delete the supplied one.

It would be awesome if you can sort this out.

dixiekong avatar Oct 10 '22 19:10 dixiekong

I'm using patch-package so that I can continue to use @types/flexsearch by deleting the provided index.d.ts file.

darrenmothersele avatar Oct 10 '22 19:10 darrenmothersele

Same problem here when I try to create a Document index with version 0.7.31. When importing like import {Document} from 'flexsearch' the provided typings point to:

  interface Document {
    id: string;
    field: any;
  }

But it should point to something like this

BasLee avatar Oct 11 '22 11:10 BasLee

After updating from 0.7.21 to 0.7.31 it's completely unclear how to import Document and Index. Is there a guide or changelog to go through?

For now that feels like a breaking change

klimashkin avatar Oct 19 '22 00:10 klimashkin

After updating from 0.7.21 to 0.7.31 it's completely unclear how to import Document and Index. Is there a guide or changelog to go through?

For now that feels like a breaking change

had to fork it - https://www.npmjs.com/package/flexsearch-ts and swap types with @types/flexsearch happy to PR if others find this useful...

frankleng avatar Oct 20 '22 16:10 frankleng

I'm experiencing this issue after updating from v0.7.21 to v0.7.31 (I was using this lib with @types/... definitions before). As of right now it seems completely unusable with typescript (I mean, it works once compiled but it throws type errors everywhere on dev time).

I had to rollback to 0.7.21 for the time being.

lucas-labs avatar Oct 26 '22 10:10 lucas-labs

Same issue after upgrading to 0.7.31. The DefinitelyTyped version (which is arguably the correct typing) is being shadowed by the built-in typing at node_modules/flexsearch/index.d.ts. I have to downgrade to 0.7.21.

In the meanwhile, @ts-thomas (author of Flexsearch) and @Losses (Author of de-facto the best typing in DefinitelyTyped project), would you consider bringing the DefinitelyTyped version into this repo? The typing files won't hurt performance and it's in theory a O(1) complexity change with huge improvement for developers who expect out-of-box typing. The competing library Lyra search, for example, already ships with full typescript support.

chuanqisun avatar Nov 03 '22 06:11 chuanqisun

I also saw #343 trying to overhaul the typing. There is probably conflict change. Would it help or make it worse if we break that into a separate typing update plus other bigger changes for later, just to unblock existing typescript users? Just throwing some ideas out there.

chuanqisun avatar Nov 03 '22 06:11 chuanqisun

Thanks for this report. The whole ts-type dilemma is pretty strange. This is the only part of code which wasn't contributed by me. I really would like to have some advice, which of the type definitions are the proper one and what needs to be changed. If someone could creating a pull request, that would be great.

Thanks a lot.

ts-thomas avatar Nov 04 '22 19:11 ts-thomas

Same issue after upgrading to 0.7.31. The DefinitelyTyped version (which is arguably the correct typing) is being shadowed by the built-in typing at node_modules/flexsearch/index.d.ts. I have to downgrade to 0.7.21.

In the meanwhile, @ts-thomas (author of Flexsearch) and @Losses (Author of de-facto the best typing in DefinitelyTyped project), would you consider bringing the DefinitelyTyped version into this repo? The typing files won't hurt performance and it's in theory a O(1) complexity change with huge improvement for developers who expect out-of-box typing. The competing library Lyra search, for example, already ships with full typescript support.

I'd like to if the developers of this library agree to merge the pull request... https://github.com/nextapps-de/flexsearch/pull/266

Losses avatar Nov 05 '22 11:11 Losses

For all of those struggling with this using pnpm here is how to install the @types/flexsearch and overwrite the package types.

  1. pnpm add @types/flexsearch
  2. pnpm patch flexsearch
  3. open the folder given by patch... remove index.d.ts
  4. pnpm patch-commit [folder given in step 2]

nickreese avatar Dec 04 '22 12:12 nickreese

Up

itPiligrim avatar Dec 25 '22 17:12 itPiligrim

Up.

rap2hpoutre avatar Jan 27 '23 16:01 rap2hpoutre

For all of those struggling with this using pnpm here is how to install the @types/flexsearch and overwrite the package types.

  1. pnpm add @types/flexsearch
  2. pnpm patch flexsearch
  3. open the folder given by patch... remove index.d.ts
  4. pnpm patch-commit [folder given in step 2]
  1. Restart VS-Code, and then it will work it out

yenche123 avatar Feb 14 '23 12:02 yenche123

this fixes he issue for me (workarround only)

npm i -D flexsearch
npm i -D @types/flexsearch
rm -rf node_modules/flexsearch/index.d.ts

regnete avatar Mar 13 '23 08:03 regnete

Just came here wondering if the library was dropped because of how broken the type definitions are. I believe it would be much better to ship without types (just delete index.d.ts) for the sake of not sending wrong signals about the status of the project.

fgarcia avatar Mar 15 '23 11:03 fgarcia

100% agree with @fgarcia here. If the official type definitions are not going to be actively maintained but the library is then at least remove them so @types/flexsearch can be managed by the community.

NuroDev avatar Apr 06 '23 08:04 NuroDev

@ts-thomas would be great if you could remove the index.d.ts from the package/release

basically merging this PR: https://github.com/nextapps-de/flexsearch/pull/366

OneCyrus avatar Apr 11 '23 06:04 OneCyrus

@ts-thomas would be great if you could remove the index.d.ts from the package/release

basically merging this PR: #366

As the author of @types/flexsearch, my heart breaks every time when someone replies to this issue and I received the notification. Feel my despair sir. (ㆆᴗㆆ)

Losses avatar Apr 12 '23 07:04 Losses

Bump.

Although, if I were in the shoes of @ts-thomas I would drop index.d.t.s and let the community manage the TS typings. As he has said in this thread (quoted below) he isn't sure about what should be done as the community has varying opinions on how the library should go forward in this regard, so I'd say his best bet is to drop index.d.ts

Thanks for this report. The whole ts-type dilemma is pretty strange. This is the only part of code which wasn't contributed by me. I really would like to have some advice, which of the type definitions are the proper one and what needs to be changed. If someone could creating a pull request, that would be great.

Thanks a lot.

robigan avatar Apr 28 '23 09:04 robigan

After updating from 0.7.21 to 0.7.31 it's completely unclear how to import Document and Index. Is there a guide or changelog to go through? For now that feels like a breaking change

had to fork it - https://www.npmjs.com/package/flexsearch-ts and swap types with @types/flexsearch happy to PR if others find this useful...

I am trying to use your fork and still doesn't work for me. Error below

$ npm run index

> [email protected] index
> tsc --outDir dist && cd dist && node index.js

file:///Users/mememe/Documents/Football/FlexSearchFail/dist/index.js:27
    var doc = new fst.Document({
              ^

TypeError: fst.Document is not a constructor

The code for index.ts looks like this

import * as fst from 'flexsearch-ts';
import { Player } from './player.js';

(async () =>{
    const players = [
        new Player("928fd47b", "Zlatan Ibrahimovic", "03-10-81"),
        new Player("dd76a750", "Lionel Messi", "24-06-87"),
        new Player("6177f012", "Cristiano Ronaldo", "05-02-85"),
    ];
    var doc = new fst.Document<Player, false>({
        document: {
            id: "identifier",
            index: ["name", "dob"]
        }
    });
    players.forEach(p => doc.add(p.id, p));

    doc.searchAsync("Messi").then(searchResults => {
        searchResults.forEach(sr => {
          console.log(sr.result);
        })
    });
})();

package.json:

{
  "name": "flexsearchfail",
  "type": "module",
  "version": "1.0.0",
  "description": "Repro steps to fail at using FlexSearch from a typescript node app",
  "main": "index.ts",
  "scripts": {
    "build": "tsc --outDir dist",
    "index": "tsc --outDir dist && cd dist && node index.js"
  },
  "author": "me",
  "license": "MIT",
  "devDependencies": {
    "ts-node": "^8.2.0",
    "tslint": "^5.17.0",
    "typescript": "^4.9.5"
  },
  "dependencies": {
    "flexsearch-ts": "^0.7.35"
  }
}

romanfq avatar Oct 12 '23 13:10 romanfq