-
Notifications
You must be signed in to change notification settings - Fork 429
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: support search weight configuration for slug fields #7852
base: next
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
No changes to documentation |
Component Testing Report Updated Nov 20, 2024 2:32 PM (UTC) ❌ Failed Tests (1) -- expand for details
|
⚡️ Editor Performance ReportUpdated Wed, 20 Nov 2024 14:34:43 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
8e419cb
to
6d3653d
Compare
6d3653d
to
6cc7b43
Compare
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.
Just some comments to get some clarification before ✅ 🙏
const isSearchConfiguration = (options: unknown): options is SearchConfiguration => | ||
isRecord(options) && 'search' in options && isRecord(options.search) | ||
|
||
function isSchemaType(input: SchemaType | CrossDatasetType | undefined): input is SchemaType { | ||
return typeof input !== 'undefined' && 'name' in input | ||
} | ||
|
||
function getFullyQualifiedPath(schemaType: SchemaType, path: string): string { |
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.
Maybe we need to add a comment or maybe change the name? I'm just not sure what "fully qualified" would mean in this context 🥲
Could it be simplified to getFullPath
or something similar?
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.
Or, since it seems to just be used when the field is a Slug
as per line 83 maybe make it related to that? 🤔
Description
options.search.weight
option. It was previously not permitted by the types, nor had any impact if set.source
field, resulting in slightly smaller search queries. Matching only occurs on the slug object'scurrent
field.Global Search Example
Imagine two different document types:
book
contains atitle
field that is implicitly given a weight of10
because it's selected as the list preview title.author
contains aslug
field that is explicitly given a weight of1_000
via theoptions.search.weight
option.next
fix/sapp-34/slug-field-weights
next
fix/sapp-34/slug-field-weights
Both
book.title
andauthor.slug
contain an exact match for the term"xoxoxo"
. With the changes introduced in this branch, the weighting set forauthor.slug
is respected, and the author document is therefore recognised as more relevant than the book document.What to review
options.search.weight
on a slug (or slug-based/aliased) field.Testing
packages/sanity/src/core/search/common/__tests__/deriveSearchWeightsFromType.test.ts
.Notes for release
It's now possible to configure search weights for slug fields using the
options.search.weight
field option:The slug field's weight will now be respected when using global search with "Best match" sorting selected, and when using reference search.