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 inference for the returned values of search #538

Closed
hackpirodev opened this issue Oct 30, 2023 · 7 comments
Closed

Incorrect type inference for the returned values of search #538

hackpirodev opened this issue Oct 30, 2023 · 7 comments

Comments

@hackpirodev
Copy link

hackpirodev commented Oct 30, 2023

Describe the bug
the return type of the search's values are inferred by schema, instead imo should be inferred by the document. Considering how works orama:
"The schema represents the searchable properties of the document to be inserted. Not all properties need to be indexed, but only those that we want to be able to search for."

To Reproduce
https://stackblitz.com/edit/vitejs-vite-qpmtfg?file=src%2Fmain.ts,package.json&terminal=dev
here as result documents are inferred only the keys passed as schema and the other values are solved as any.

const results = await search(myDb, searchParams);

Expected behavior

type AgendaDocument = TypedDocument<OramaAgenda>;
const results: AgendaDocument =  await search(myDb, searchParams);
@micheleriva
Copy link
Member

Quick question, when defining the schema, are you using as const?

const db = await create({
  schema: {
    title: 'string',
    description: 'string'
  } as const // <--- here
})

@hackpirodev
Copy link
Author

@micheleriva yes, in the provided example (from line 33) I declared this schema =>

const schema = {
  conference: 'string',
  talk: 'string',
  city: 'string',
} as const;

const myDb = await create({
  schema,
});

@allevo
Copy link
Collaborator

allevo commented Oct 31, 2023

Hi!
I changed the example above in this way: https://codesandbox.io/s/epic-star-l4phcy
If I can, do you think the documentation here needs to be revised? Just to understand if we should improve (and how) our documentation.
Any feedback is welcome.

@hackpirodev
Copy link
Author

Hi @allevo! Thank you for responding to me. I didn't find a solution in the provided link.

About the docs If I don't misunderstand how Orama works, maybe I'd like have something like this:

import type { TypedDocument, Orama, Results, SearchParams } from '@orama/orama'
import { create, insert, search } from '@orama/orama'
 
const movieSchema = {
  title: 'string',
  year: 'number',
  actors: 'string[]',
  isFavorite: 'boolean',
  stars: 'enum' // <-- delete this property If doesn't exist on the documents inserted (in this document) Why would I want to have to index it?
} as const // <-- this is important
type MovieDocument = TypedDocument<Orama<typeof movieSchema>>  // <== Why is MovieDocument  take from schema?  shouldn't it be taken from the type of documents I insert?
 
const movieDB: Orama<typeof movieSchema> = await create({
  schema: movieSchema,
})
 
const idP: string = await insert(movieDB, {
  title: 'The Godfather',
  year: 1972,
  actors: ['Marlon Brando', 'Al Pacino'],
  isFavorite: true,
  imdbLink: "https://www.imdb.com/title/tt0068646/" // < -- Add one property that it's not indexed
})
 
const searchParams: SearchParams<Orama<typeof movieSchema>> = {
  term: 'godfather',
}
const result: Results<MovieDocument> = await search(movieDB, searchParams)
const title = result.hits[0].document.imdbLink // well typed!

If I understand correctly, assuming I change the example appropriately, this would be my case.

I apologize if my link contained too much noise. Let me explain the issue more clearly:
minimale case stackblitz
While reading the documentation, I discovered that when creating a new database in Orama, I need to pass a schema, which defines the properties I want to index. In this specific case, I want Orama to index only the 'conference' property:

const schema = {
  conference: 'string',
} as const;

const myDb = await create({
  schema,
});

But I also want to be able to add items with more properties than specified in my schema. For example:

const agenda = [
  {
    date: '14/04/2023',
    conference: 'Node Congress',
    link: 'https://nodecongress.com/',
  },
  {
    date: '12/05/2023',
    conference: 'AppDevCon',
    link: 'https://appdevcon.nl/',
  },
];

type Agenda = typeof agenda[number];

I can insert my documents inside Orama like this:

await insertMultiple(myDb, agenda)

If I perform a search with an empty string, I expect to retrieve all documents typed as Results

const searchParams: SearchParams<MyOramaSchema> = {
  term: '',
};

type ExpectedReturn =  Results<Agenda>
const results = await search(myDb, searchParams)

Orama seems to only infer the schema properties, and in this case, 'link' is resolved as 'any':
image
but as you can see the document it's returned complete as expected
image

The only workaround I found for this issue is to cast the result to obtain the document type properly:

const results = await search(myDb, searchParams) as unknown as Results<Agenda>

now it's typed properly =>
image

@allevo
Copy link
Collaborator

allevo commented Oct 31, 2023

Oki, I understood the point.
Because the schema contains just a few properties, the returned documents are typed partially only with the properties defined inside the schema. The remaining properties are typed as any.

For now, Orama has yet to learn what the inserted document looks like and infer the type from the schema. Orama needs to understand which property has which type due to internal calculation (different trees, different where conditions, and so on).
Instead, the remaining properties are completely ignored by Orama because they are never read.
That means an unlisted property could have different types of objects.

Because we want to support the last described case, deducting the returned document type could be problematic when objects having different shapes are added to Orama.

Your proposed fix is correct because Orama cannot understand how the document looks like a priori.

@micheleriva WDYT? Should we support this case?

@micheleriva
Copy link
Member

@allevo what you're saying is correct. We could pass a second, optional type argument to the Results type to help the compiler match additional property types... that's the only solution I can think of.

@allevo
Copy link
Collaborator

allevo commented Nov 2, 2023

I found a solution that avoids the as keyword.
In the following example, the indexed property is just title, but the documents also contain year. You can strongly type the returned document as below:

const movieSchema = {
  title: 'string',
} as const
const db = await create({ schema: movieSchema })

interface Movie {
  title: string,
  year: number,
}

//             this is important ---v 
const r = await search<typeof db, Movie>(db, { term: '' })
const title = r.hits[0].document.title // well typed!
const year = r.hits[0].document.year // well typed!

I'll update the documentation as well.
Thanks a lot for your patience.

allevo added a commit that referenced this issue Nov 2, 2023
allevo added a commit that referenced this issue Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants