Skip to content
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

Incorrect type definitions #342

Open
dixiekong opened this issue Oct 10, 2022 · 20 comments · May be fixed by #364
Open

Incorrect type definitions #342

dixiekong opened this issue Oct 10, 2022 · 20 comments · May be fixed by #364
Labels
help wanted Extra attention is needed

Comments

@dixiekong
Copy link

dixiekong commented Oct 10, 2022

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.

@darrenmothersele
Copy link

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

@BasLee
Copy link

BasLee commented Oct 11, 2022

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

@ignatiusmb ignatiusmb mentioned this issue Oct 12, 2022
4 tasks
erikhofer added a commit to scope42/scope42 that referenced this issue Oct 15, 2022
@klimashkin
Copy link

klimashkin commented Oct 19, 2022

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

@frankleng
Copy link

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...

@lucas-labs
Copy link

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.

@chuanqisun
Copy link

chuanqisun commented Nov 3, 2022

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
Copy link

chuanqisun commented Nov 3, 2022

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.

@ts-thomas
Copy link
Contributor

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 ts-thomas added the help wanted Extra attention is needed label Nov 4, 2022
@Losses
Copy link

Losses commented Nov 5, 2022

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...
#266

@nickreese
Copy link

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]

@itPiligrim
Copy link
Contributor

Up

@samuba samuba linked a pull request Jan 2, 2023 that will close this issue
@rap2hpoutre
Copy link

Up.

@yenche123
Copy link

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

@regnete
Copy link

regnete commented Mar 13, 2023

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

@fgarcia
Copy link

fgarcia commented Mar 15, 2023

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.

@NuroDev
Copy link

NuroDev commented Apr 6, 2023

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.

@OneCyrus
Copy link

OneCyrus commented Apr 11, 2023

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

basically merging this PR: #366

@Losses
Copy link

Losses commented Apr 12, 2023

@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. (ㆆᴗㆆ)

@robigan
Copy link

robigan commented Apr 28, 2023

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.

@romanfq
Copy link

romanfq commented Oct 12, 2023

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"
  }
}

alecarn added a commit to infra-geo-ouverte/igo2-lib that referenced this issue Dec 19, 2023
* fix(geo): do not upgrade flexsearch since it seem not compatible with TS
nextapps-de/flexsearch#342
pelord pushed a commit to infra-geo-ouverte/igo2-lib that referenced this issue Dec 27, 2023
* feat: update Angular v17

* fix(geo): do not upgrade flexsearch since it seem not compatible with TS
nextapps-de/flexsearch#342

* fix(core/language): TranslateModule call forRoot only once with provideRootTranslation

- IgoLanguageModule only export the TranslateModule

BREAKING CHANGE: IgoLanguageModule don't import TranslateModule.forRoot
defaultLanguageLoader is not exported anymore
- provideDefaultLanguageLoader and provideLanguageLoader are replaced by DEFAULT_LANGUAGE_LOADER and set directly inside the TranslationConfig

* fix(auth): test
pelord pushed a commit to infra-geo-ouverte/igo2-lib that referenced this issue Jan 18, 2024
* feat: update Angular v17

* fix(geo): do not upgrade flexsearch since it seem not compatible with TS
nextapps-de/flexsearch#342

* feat(demo): Convert all components, directives and pipes to standalone

* feat(demo): Remove unnecessary NgModule classes

* fix(core/language): TranslateModule call forRoot only once with provideRootTranslation

- IgoLanguageModule only export the TranslateModule

BREAKING CHANGE: IgoLanguageModule don't import TranslateModule.forRoot
defaultLanguageLoader is not exported anymore
- provideDefaultLanguageLoader and provideLanguageLoader are replaced by DEFAULT_LANGUAGE_LOADER and set directly inside the TranslationConfig

* feat: remove extra module and rework navigation

* fix(demo): main clean up

* fix(test)

* fix(workspace): code deleted by error
alecarn added a commit to infra-geo-ouverte/igo2-lib that referenced this issue Feb 19, 2024
* fix(geo): do not upgrade flexsearch since it seem not compatible with TS
nextapps-de/flexsearch#342

* fix(core): translatemodule forRoot is now called once (#1551)

* feat: update Angular v17 (#1543)

* feat(core/language): refact the language initialization (#1563)

BREAKING CHANGE: IgoLanguageModule don't import TranslateModule.forRoot
defaultLanguageLoader is not exported anymore
- provideDefaultLanguageLoader and provideLanguageLoader are replaced by DEFAULT_LANGUAGE_LOADER and set directly inside the TranslationConfig

* feat(demo): Convert all components, directives and pipes to standalone

* refactor(demo): Remove unnecessary NgModule classes, visual adjustment and rework navigation (#1556)

* fix(tsconfig): remove deprecated config and unnecessary (#1610)

* fix(all): resolve template type checking

BREAKING CHANGE ToolboxColor is now a type

* fix(auth): remove unused module (#1619)

BREAKING CHANGES

* feat(ci): update pipeline

* refact(auth): missing deps due to merge conflict

* fix(demo): geo/simple-map missing optionnal chaining
alecarn added a commit to infra-geo-ouverte/igo2-lib that referenced this issue Mar 8, 2024
* fix(geo): do not upgrade flexsearch since it seem not compatible with TS
nextapps-de/flexsearch#342
alecarn added a commit to infra-geo-ouverte/igo2-lib that referenced this issue Mar 8, 2024
* feat: update Angular v17

* fix(geo): do not upgrade flexsearch since it seem not compatible with TS
nextapps-de/flexsearch#342

* feat(demo): Convert all components, directives and pipes to standalone

* feat(demo): Remove unnecessary NgModule classes

* fix(core/language): TranslateModule call forRoot only once with provideRootTranslation

- IgoLanguageModule only export the TranslateModule

BREAKING CHANGE: IgoLanguageModule don't import TranslateModule.forRoot
defaultLanguageLoader is not exported anymore
- provideDefaultLanguageLoader and provideLanguageLoader are replaced by DEFAULT_LANGUAGE_LOADER and set directly inside the TranslationConfig

* feat: remove extra module and rework navigation

* fix(demo): main clean up

* fix(test)

* fix(workspace): code deleted by error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.