-
Notifications
You must be signed in to change notification settings - Fork 187
fix(db-ivm): hash Temporal objects by value instead of identity #1368
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@tanstack/db-ivm': patch | ||
| --- | ||
|
|
||
| Fix Temporal objects (PlainDate, ZonedDateTime, etc.) producing identical hashes in the IVM hash function. Temporal objects have no enumerable own properties, so Object.keys() returns [] and all instances were hashed identically. This caused join live queries to silently swallow updates when only a Temporal field changed. Temporal objects are now hashed by their type tag and string representation. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,15 @@ import { describe, expect, it } from 'vitest' | |
| import { DefaultMap } from '../src/utils.js' | ||
| import { hash } from '../src/hashing/index.js' | ||
|
|
||
| // Minimal mock that mimics Temporal objects: Symbol.toStringTag + toString() | ||
| // without requiring the temporal-polyfill dependency. | ||
| function createTemporalLike(tag: string, value: string) { | ||
| return Object.create(null, { | ||
| [Symbol.toStringTag]: { value: tag }, | ||
| toString: { value: () => value }, | ||
| }) | ||
| } | ||
|
Comment on lines
+7
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -type f -name "utils.test.ts" | grep -E "db-ivm|packages"Repository: TanStack/db Length of output: 126 🏁 Script executed: cd packages/db-ivm && git ls-files | grep -E "test|utils"Repository: TanStack/db Length of output: 1090 🏁 Script executed: # Get the file and check its existence and size
wc -l packages/db-ivm/tests/utils.test.ts 2>/dev/null || echo "File not found, trying alternative paths"
find . -type f -name "utils.test.ts" -path "*db-ivm*"Repository: TanStack/db Length of output: 132 🏁 Script executed: find . -type f -name "*.test.ts" 2>/dev/null | head -20Repository: TanStack/db Length of output: 1072 🏁 Script executed: # Look for db-ivm directory structure
fd -t d db-ivm 2>/dev/nullRepository: TanStack/db Length of output: 71 🏁 Script executed: # List top-level packages
ls -la packages/ 2>/dev/null || echo "packages dir not found"Repository: TanStack/db Length of output: 1120 🏁 Script executed: head -30 packages/db-ivm/tests/utils.test.tsRepository: TanStack/db Length of output: 975 🏁 Script executed: sed -n '1,20p' packages/db-ivm/tests/utils.test.tsRepository: TanStack/db Length of output: 729 🏁 Script executed: sed -n '190,210p' packages/db-ivm/tests/utils.test.tsRepository: TanStack/db Length of output: 984 🏁 Script executed: # Verify what hashTemporal actually does - search for the hash function implementation
rg -A 10 "hashTemporal|Symbol.toStringTag" packages/db-ivm/src --type tsRepository: TanStack/db Length of output: 1887 🏁 Script executed: # Check if there are any other uses of createTemporalLike to understand the broader context
rg "createTemporalLike" packages/db-ivm/tests/utils.test.ts -B 2 -A 2Repository: TanStack/db Length of output: 1619 Add explicit return type to helper and tighten cross-type test. The Typed helper and isolated tag-checking test+type TemporalLike = {
+ readonly [Symbol.toStringTag]: string
+ toString(): string
+}
+
-function createTemporalLike(tag: string, value: string) {
+function createTemporalLike(tag: string, value: string): TemporalLike {
return Object.create(null, {
[Symbol.toStringTag]: { value: tag },
toString: { value: () => value },
- })
+ }) as TemporalLike
}
@@
- const plainDate = createTemporalLike(`Temporal.PlainDate`, `2024-01-15`)
+ const sharedValue = `same-value`
+ const plainDate = createTemporalLike(`Temporal.PlainDate`, sharedValue)
const plainDateTime = createTemporalLike(
`Temporal.PlainDateTime`,
- `2024-01-15T00:00:00`,
+ sharedValue,
)🤖 Prompt for AI Agents |
||
|
|
||
| describe(`DefaultMap`, () => { | ||
| it(`should return default value for missing keys`, () => { | ||
| const map = new DefaultMap(() => 0) | ||
|
|
@@ -170,6 +179,53 @@ describe(`hash`, () => { | |
| expect(hash1).not.toBe(hash3) // Different dates should have different hash | ||
| }) | ||
|
|
||
| it(`should hash Temporal objects by value`, () => { | ||
| const date1 = createTemporalLike(`Temporal.PlainDate`, `2024-01-15`) | ||
| const date2 = createTemporalLike(`Temporal.PlainDate`, `2024-01-15`) | ||
| const date3 = createTemporalLike(`Temporal.PlainDate`, `2024-06-15`) | ||
|
|
||
| const hash1 = hash(date1) | ||
| const hash2 = hash(date2) | ||
| const hash3 = hash(date3) | ||
|
|
||
| expect(typeof hash1).toBe(hashType) | ||
| expect(hash1).toBe(hash2) // Same Temporal date should have same hash | ||
| expect(hash1).not.toBe(hash3) // Different Temporal dates should have different hash | ||
|
|
||
| // Different Temporal types with overlapping string representations should differ | ||
| const plainDate = createTemporalLike(`Temporal.PlainDate`, `2024-01-15`) | ||
| const plainDateTime = createTemporalLike( | ||
| `Temporal.PlainDateTime`, | ||
| `2024-01-15T00:00:00`, | ||
| ) | ||
|
|
||
| expect(hash(plainDate)).not.toBe(hash(plainDateTime)) | ||
|
|
||
| // Other Temporal types should also hash correctly | ||
| const time1 = createTemporalLike(`Temporal.PlainTime`, `10:30:00`) | ||
| const time2 = createTemporalLike(`Temporal.PlainTime`, `10:30:00`) | ||
| const time3 = createTemporalLike(`Temporal.PlainTime`, `14:00:00`) | ||
|
|
||
| expect(hash(time1)).toBe(hash(time2)) | ||
| expect(hash(time1)).not.toBe(hash(time3)) | ||
|
|
||
| const instant1 = createTemporalLike( | ||
| `Temporal.Instant`, | ||
| `2024-01-15T00:00:00Z`, | ||
| ) | ||
| const instant2 = createTemporalLike( | ||
| `Temporal.Instant`, | ||
| `2024-01-15T00:00:00Z`, | ||
| ) | ||
| const instant3 = createTemporalLike( | ||
| `Temporal.Instant`, | ||
| `2024-06-15T00:00:00Z`, | ||
| ) | ||
|
|
||
| expect(hash(instant1)).toBe(hash(instant2)) | ||
| expect(hash(instant1)).not.toBe(hash(instant3)) | ||
| }) | ||
|
|
||
| it(`should hash RegExp objects`, () => { | ||
| const regex1 = /test/g | ||
| const regex2 = /test/g | ||
|
|
||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: TanStack/db
Length of output: 288
🏁 Script executed:
Repository: TanStack/db
Length of output: 1804
🏁 Script executed:
Repository: TanStack/db
Length of output: 2053
🏁 Script executed:
rg "isTemporal|hashTemporal" packages/db-ivm/src/hashing/hash.ts -A 2 -B 2Repository: TanStack/db
Length of output: 806
🏁 Script executed:
Repository: TanStack/db
Length of output: 94
🏁 Script executed:
Repository: TanStack/db
Length of output: 7580
🏁 Script executed:
Repository: TanStack/db
Length of output: 276
🏁 Script executed:
Repository: TanStack/db
Length of output: 1276
🏁 Script executed:
Repository: TanStack/db
Length of output: 1640
🏁 Script executed:
Repository: TanStack/db
Length of output: 2221
🏁 Script executed:
Repository: TanStack/db
Length of output: 82
🏁 Script executed:
Repository: TanStack/db
Length of output: 82
🏁 Script executed:
Repository: TanStack/db
Length of output: 428
🏁 Script executed:
Repository: TanStack/db
Length of output: 3975
🏁 Script executed:
Repository: TanStack/db
Length of output: 209
🏁 Script executed:
grep -n "as any" packages/db-ivm/src/hashing/hash.tsRepository: TanStack/db
Length of output: 163
🏁 Script executed:
Repository: TanStack/db
Length of output: 1514
🏁 Script executed:
rg "function hashTemporal|function hash\(" packages/db-ivm/src/hashing/hash.ts -A 8Repository: TanStack/db
Length of output: 619
Guard the Temporal branch before calling
toString().isTemporal()currently only checksSymbol.toStringTag. A null-prototype object that spoofsTemporal.PlainDate(e.g., viaObject.create(null)) will pass the tag check but throw whenhashTemporal()callstoString()on line 132, since null-prototype objects do not inherittoString. Replace theanycasts withunknownand add a type guard to verifytoStringis callable before hashing:Safer type-guarding for the Temporal path
Per coding guidelines: "Avoid using
anytypes; useunknowninstead when type is truly unknown and provide type guards to narrow safely."📝 Committable suggestion
🤖 Prompt for AI Agents