-
-
Notifications
You must be signed in to change notification settings - Fork 297
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
feat: added strong typed orama schema #470
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Hi @H4ad , thanks! Would you be able to fix conflicts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this mostly looks good to me. I think we will need to update the docs as well if we want to use as const
when defining the schema. But I really with we could get rid of it actually... so we can avoid such a drastic change.
@H4ad will this work on TypeScript <5?
@allevo could you also take a look at this?
@micheleriva I'm still working on types, and cleaning the tests (some of them passes but the types is actually broken). It could work on <5 but I don't think we will get the same results and API that we have now since |
Now the code is stable enough, I also added better types for search (I missed some opportunities to correctly type), so now I think the types are good enough. |
If I understood correctly, this PR propagates the schema definition to the insert/insertMultiple/search methods: amazing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should update the documentation as well
Something like "orama is fully typed so the document type is inferred from the schema" in create, insert, and search pages.
WDYT?
@allevo I have no skills to write good documentation about this, I think the documentation should be in another PR as we did for internal IDs, since this is a breaking change and also very important, this probably will deserve a blog post too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I'll care about docs! No problem.
LGTM
const s = await search(db, { term: 'fish' }) | ||
const year: any = s.hits[0].document.year // not inferred, so it's any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not inferred but you can make it type-safe by passing the interface with that field, like: search<WithYearField>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. I'm changing it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a test is missing for this case. Could you add it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests that you have on Orama don't test for types, you can literally break the types and the tests will not break, they only break when the assert is false (at least that was the behavior I had when I was changing the Orama tests).
But I can add it by the end of the day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, till now, we don't have that kind of test. But with this PR, type types are guaranteed. So, a little test on that would be amazing!
At least, we document this capability in the test suite also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, we can use: https://github.com/SamVerschueren/tsd
Is the same lib used by https://github.com/sindresorhus/type-fest/blob/main/test-d/abstract-class.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But probably this will require a new project and command to test only the types, separated from the default tap tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tsd
seems to do the job greatly.
I'll go for just a new command run after the test suite. it should work.
packages/docs/pages/index.mdx
Outdated
@@ -19,40 +19,6 @@ Orama is a fast, batteries-included, full-text and vector search engine entirely | |||
|
|||
Get started with just a few lines of code: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@allevo please remove this line as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just a little fix for @allevo
Hi @H4ad, we only need a few conflict fixes to get this one merged. Thanks! |
@micheleriva @allevo what should be the type for enum? string, number? both? |
Based on the tests, I leave it as And we can merge this PR before #482, I don't want to resolve conflicts for the third time 😓 |
@H4ad you're right. Sorry if it took so long for us to merge, it's a pretty complex one. Thanks for your work! |
Thanks! |
Closes #456
/claim #456
This PR turns the Orama completely type-safe, below some examples of how to use:
Schema
Now, if you want to have the type from the result of your schema, you can do the following:
Be careful with
as const
, it is required to be able to correctly infer your schema, if you didn't make it const, your document will be inferred tonever
.Create
By default, create will create the correct typings from schema automatically:
The return type will be:
It also works for more complex schemas:
If you didn't customize the index, sorter, or documentStore, it will infer to the default implementations.
Insert & Update
During the insert, all the properties are optional and you can also specific custom properties, like:
But, if you specify a wrong value for some value in the schema, for example passing a number to quote, you will receive an error:
Search
With search, you will have two behaviors, by default the result type will be the typed-schema:
But, in cases where the result will be too much different from the typed-schema, you can do the following:
Also, now we automatically suggest options for
groupBy.properties
,sortBy.property
andwhere
, like this:Currently, nested properties such as
doc.title
are not suggested even if you have schema support, but you can specify them without problems.Sharing Typed Orama Instance
In cases where you need the type for Orama, you can do the following:
Hooks
The hooks from
components
also are correctly typed based on the schema:But is not possible to infer extra properties, like when your schema has just
title
but in your document you also insertdescription
.In those cases, you should do the following:
But nothing stops you from just grab the property you want, like:
It just will just be typed as
any
.Breaking Changes
I needed to upgrade the typescript to 5.0.0 because of the new modifier
const
, this makes it easier for the users to specify the schema without the need of doing{ schema: { title: 'string' } as const }
.