-
Notifications
You must be signed in to change notification settings - Fork 159
Make indexing opt-in #950
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
base: main
Are you sure you want to change the base?
Make indexing opt-in #950
Conversation
🦋 Changeset detectedLatest commit: f2796da The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: +1.94 kB (+2.14%) Total Size: 92.5 kB
ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 3.7 kB ℹ️ View Unchanged
|
kevin-dp
left a comment
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 left some comments that i would like to see addressed. Nothing major. There are no unit tests for the new BasicIndex. We should add unit tests.
| // Dev mode detection settings - ON by default in non-production | ||
| let devModeConfig: IndexDevModeConfig = { | ||
| enabled: true, | ||
| collectionSizeThreshold: 1000, |
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.
This threshold seems arbitrary. Going through an non-indexed collection of 1000 rows should be pretty fast. I guess modern machines can easily go to perhaps 1M rows before it really becomes slow and needs indexes. Time-based suggestions (slowQueryThresholdMs) makes much more sense imo.
6df29a2 to
79db466
Compare
This POC explores making indexing a tree-shakeable opt-in feature to reduce
default bundle size. Key changes:
- Create new `@tanstack/db/indexing` entry point for BTreeIndex and related utils
- Add index registry to replace hard-coded BTreeIndex defaults
- Add dev-mode auto-detection for when indexes would help (collection size,
slow queries)
- Remove BTreeIndex value export from main entry (types still exported)
Bundle size improvements when NOT using indexing:
- Minified: ~15 KB saved (6.9%)
- Gzipped: ~5.4 KB saved (8.5%)
Usage after this change:
```ts
// Option 1: Enable indexing globally
import { enableIndexing } from '@tanstack/db/indexing'
enableIndexing()
// Option 2: Use explicit index type (best for tree-shaking)
import { BTreeIndex } from '@tanstack/db/indexing'
collection.createIndex((row) => row.userId, { indexType: BTreeIndex })
// Dev mode for index suggestions
import { configureIndexDevMode } from '@tanstack/db'
configureIndexDevMode({ enabled: true, collectionSizeThreshold: 100 })
```
Note: Additional savings (~25KB more) possible by making index-optimization.ts
lazy-loaded, but would require more extensive refactoring of change-events.ts
and order-by.ts.
…y default
Breaking change: autoIndex now defaults to 'off' instead of 'eager'.
This reduces default bundle size by not requiring indexing code.
Changes:
- autoIndex defaults to 'off' - users must explicitly enable or add indexes
- Dev mode suggestions are ON by default (in non-production) to help
developers identify when indexes would improve performance
- Updated tests to reflect new default behavior
Users who want auto-indexing can either:
1. Set autoIndex: 'eager' on individual collections
2. Import and register BTreeIndex globally:
```ts
import { enableIndexing } from '@tanstack/db/indexing'
enableIndexing()
```
Dev mode will warn in console when queries could benefit from indexes,
and this will also be available in devtools.
Most collections won't have perf issues until much larger sizes. Slow query detection (10ms) is the more actionable metric.
- Add MapIndex for equality lookups (eq, in) without BTree overhead - Change enableIndexing() to use MapIndex by default - Add enableBTreeIndexing() for ORDER BY optimization on large collections - BTreeIndex is only needed for sorted iteration (10k+ items with ORDER BY) Bundle size impact: - MapIndex: ~5 KB (~1.3 KB gzipped) - BTreeIndex: ~33 KB (~7.8 KB gzipped) - Savings with MapIndex: ~27 KB (~6 KB gzipped)
…ndexType - Add BasicIndex using Map + sorted Array for both equality and range queries - Remove registry pattern - pass defaultIndexType to collection constructor instead - Remove lazy index infrastructure (LazyIndexWrapper, IndexProxy) - Simplify indexing.ts entry point to just export BasicIndex and BTreeIndex - Update all tests to explicitly set defaultIndexType where needed - Update changeset to reflect simplified approach Breaking changes: - createIndex() requires defaultIndexType on collection or indexType in config - enableIndexing()/enableBTreeIndexing() removed, use defaultIndexType instead
feaba11 to
ddcb1d7
Compare
- Remove duplicate findInsertionIndex method from BasicIndex - Add findInsertPositionInArray helper for simple arrays - Add deleteInSortedArray helper for deleting from sorted arrays - Keep findInsertPosition for tuple arrays (backward compatibility) - Update BasicIndex to use the new shared utility functions Co-authored-by: Kevin <[email protected]>
Replace three-iteration approach (spread, reverse, map) with a single backwards for loop for better performance. Co-authored-by: Kevin <[email protected]>
fe9b441 to
50a5fa2
Compare
kevin-dp
left a comment
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 addressed my own review.
…Fe1MCKR6TXmrXaPSEXRx
|
/review |
1 similar comment
|
/review |
🔍 Comprehensive PR Review: Make Indexing Opt-InPR: #950 | Branch: This PR makes indexing opt-in for better tree-shaking, introduces a new 📊 Summary
🔴 Critical Issues (Must Fix)1. Silent Failure in Index
|
Researched JavaScript full-text search libraries that could integrate with TanStack DB's indexing architecture following PR #950's opt-in pattern. Key findings: - Orama recommended (2KB, used by tanstack.com, full features) - MiniSearch as alternative (7KB, class-based API) - FlexSearch for high-performance needs - Custom inverted index for minimal bundle Includes implementation approach and bundle size comparison.
No description provided.