diff --git a/packages/optimistic/package.json b/packages/optimistic/package.json index 39bbf44..2d2514d 100644 --- a/packages/optimistic/package.json +++ b/packages/optimistic/package.json @@ -3,7 +3,7 @@ "description": "Core optimistic updates library", "version": "0.0.3", "dependencies": { - "@electric-sql/d2ts": "^0.1.4", + "@electric-sql/d2ts": "^0.1.5", "@standard-schema/spec": "^1.0.0", "@tanstack/store": "^0.7.0" }, diff --git a/packages/optimistic/src/collection.ts b/packages/optimistic/src/collection.ts index 614d678..3754c25 100644 --- a/packages/optimistic/src/collection.ts +++ b/packages/optimistic/src/collection.ts @@ -262,7 +262,21 @@ export class Collection> { // Create a derived array from the map to avoid recalculating it this.derivedArray = new Derived({ fn: ({ currDepVals: [stateMap] }) => { - return Array.from(stateMap.values()) + // Collections returned by a query that has an orderBy are annotated + // with the _orderByIndex field. + // This is used to sort the array when it's derived. + const array: Array = Array.from( + stateMap.values() + ) + if (array[0] && `_orderByIndex` in array[0]) { + ;(array as Array).sort((a, b) => { + if (a._orderByIndex === b._orderByIndex) { + return 0 + } + return a._orderByIndex < b._orderByIndex ? -1 : 1 + }) + } + return array }, deps: [this.derivedState], }) diff --git a/packages/optimistic/src/query/order-by.ts b/packages/optimistic/src/query/order-by.ts index e466b8b..2de068e 100644 --- a/packages/optimistic/src/query/order-by.ts +++ b/packages/optimistic/src/query/order-by.ts @@ -137,7 +137,7 @@ export function processOrderBy( } // if a and b are both booleans, compare them if (typeof a === `boolean` && typeof b === `boolean`) { - return a ? 1 : -1 + return a === b ? 0 : a ? 1 : -1 } // if a and b are both dates, compare them if (a instanceof Date && b instanceof Date) { @@ -149,11 +149,34 @@ export function processOrderBy( } // if a and b are both arrays, compare them element by element if (Array.isArray(a) && Array.isArray(b)) { - for (let i = 0; i < a.length; i++) { - const result = comparator(a[i], b[i]) - if (result !== 0) return result + for (let i = 0; i < Math.min(a.length, b.length); i++) { + // Get the values from the array + const aVal = a[i] + const bVal = b[i] + + // Compare the values + let result: number + + if (typeof aVal === `boolean` && typeof bVal === `boolean`) { + // Special handling for booleans - false comes before true + result = aVal === bVal ? 0 : aVal ? 1 : -1 + } else if (typeof aVal === `number` && typeof bVal === `number`) { + // Numeric comparison + result = aVal - bVal + } else if (typeof aVal === `string` && typeof bVal === `string`) { + // String comparison + result = aVal.localeCompare(bVal) + } else { + // Default comparison using the general comparator + result = comparator(aVal, bVal) + } + + if (result !== 0) { + return result + } } - return 0 + // All elements are equal up to the minimum length + return a.length - b.length } // if a and b are both null/undefined, return 0 if ((a === null || a === undefined) && (b === null || b === undefined)) { diff --git a/packages/optimistic/src/query/query-builder.ts b/packages/optimistic/src/query/query-builder.ts index a098d9c..9042af3 100644 --- a/packages/optimistic/src/query/query-builder.ts +++ b/packages/optimistic/src/query/query-builder.ts @@ -262,6 +262,12 @@ export class BaseQueryBuilder> { return select }) + // Ensure we have an orderByIndex in the select if we have an orderBy + // This is required if select is called after orderBy + if (this._query.orderBy) { + validatedSelects.push({ _orderByIndex: { ORDER_INDEX: `numeric` } }) + } + const newBuilder = new BaseQueryBuilder( (this as BaseQueryBuilder).query ) @@ -704,6 +710,13 @@ export class BaseQueryBuilder> { // Set the orderBy clause newBuilder.query.orderBy = orderBy + // Ensure we have an orderByIndex in the select if we have an orderBy + // This is required if select is called before orderBy + newBuilder.query.select = [ + ...(newBuilder.query.select ?? []), + { _orderByIndex: { ORDER_INDEX: `numeric` } }, + ] + return newBuilder as QueryBuilder } diff --git a/packages/optimistic/tests/query/query-builder/key-by.test.ts b/packages/optimistic/tests/query/query-builder/key-by.test.ts index afd71fc..92fe4d1 100644 --- a/packages/optimistic/tests/query/query-builder/key-by.test.ts +++ b/packages/optimistic/tests/query/query-builder/key-by.test.ts @@ -97,7 +97,13 @@ describe(`QueryBuilder.keyBy`, () => { expect(builtQuery.as).toBe(`e`) expect(builtQuery.join).toBeDefined() expect(builtQuery.where).toBeDefined() - expect(builtQuery.select).toHaveLength(3) + expect(builtQuery.select).toHaveLength(4) + expect(builtQuery.select).toEqual([ + `@e.id`, + `@e.name`, + `@d.name`, + { _orderByIndex: { ORDER_INDEX: `numeric` } }, // Added by the orderBy method + ]) expect(builtQuery.orderBy).toBe(`@e.salary`) expect(builtQuery.limit).toBe(10) expect(builtQuery.offset).toBe(5) diff --git a/packages/optimistic/tests/query/query-builder/order-by.test.ts b/packages/optimistic/tests/query/query-builder/order-by.test.ts index eb0854c..40bd578 100644 --- a/packages/optimistic/tests/query/query-builder/order-by.test.ts +++ b/packages/optimistic/tests/query/query-builder/order-by.test.ts @@ -125,7 +125,12 @@ describe(`QueryBuilder orderBy, limit, and offset`, () => { expect(builtQuery.as).toBe(`e`) expect(builtQuery.join).toBeDefined() expect(builtQuery.where).toBeDefined() - expect(builtQuery.select).toHaveLength(3) + expect(builtQuery.select).toEqual([ + `@e.id`, + `@e.name`, + `@d.name`, + { _orderByIndex: { ORDER_INDEX: `numeric` } }, // Added by the orderBy method + ]) }) }) }) diff --git a/packages/optimistic/tests/query/query-collection.test.ts b/packages/optimistic/tests/query/query-collection.test.ts index ee25413..54eb3e7 100644 --- a/packages/optimistic/tests/query/query-collection.test.ts +++ b/packages/optimistic/tests/query/query-collection.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest" +import { describe, expect, it } from "vitest" import mitt from "mitt" import { Collection } from "../../src/collection.js" import { queryBuilder } from "../../src/query/query-builder.js" @@ -40,7 +40,7 @@ const initialPersons: Array = [ name: `John Smith`, age: 35, email: `john.smith@example.com`, - isActive: true, + isActive: false, }, ] @@ -75,24 +75,19 @@ describe(`Query Collections`, () => { sync: { sync: ({ begin, write, commit }) => { // Listen for sync events - // @ts-expect-error don't trust Mitt's typing and this works. - emitter.on(`sync`, (changes: Array>) => { + emitter.on(`sync`, (changes) => { begin() - changes.forEach((change) => { + ;(changes as Array).forEach((change) => { write({ key: change.key, type: change.type, - value: change.changes, + value: change.changes as Person, }) }) commit() }) }, }, - mutationFn: async ({ transaction }) => { - emitter.emit(`sync`, transaction.mutations) - return Promise.resolve() - }, }) // Sync from initial state @@ -191,27 +186,19 @@ describe(`Query Collections`, () => { id: `person-collection-test`, sync: { sync: ({ begin, write, commit }) => { - // @ts-expect-error Mitt typing doesn't match our usage - emitter.on( - `sync-person`, - (changes: Array>) => { - begin() - changes.forEach((change) => { - write({ - key: change.key, - type: change.type, - value: change.changes, - }) + emitter.on(`sync-person`, (changes) => { + begin() + ;(changes as Array).forEach((change) => { + write({ + key: change.key, + type: change.type, + value: change.changes as Person, }) - commit() - } - ) + }) + commit() + }) }, }, - mutationFn: async ({ transaction }) => { - emitter.emit(`sync-person`, transaction.mutations) - return Promise.resolve() - }, }) // Create issue collection @@ -219,24 +206,19 @@ describe(`Query Collections`, () => { id: `issue-collection-test`, sync: { sync: ({ begin, write, commit }) => { - // @ts-expect-error Mitt typing doesn't match our usage - emitter.on(`sync-issue`, (changes: Array>) => { + emitter.on(`sync-issue`, (changes) => { begin() - changes.forEach((change) => { + ;(changes as Array).forEach((change) => { write({ key: change.key, type: change.type, - value: change.changes, + value: change.changes as Issue, }) }) commit() }) }, }, - mutationFn: async ({ transaction }) => { - emitter.emit(`sync-issue`, transaction.mutations) - return Promise.resolve() - }, }) // Sync initial person data @@ -354,8 +336,234 @@ describe(`Query Collections`, () => { // After deletion, user 3 should no longer have a joined result expect(result.state.get(`3`)).toBeUndefined() }) + + it(`should order results by specified fields`, async () => { + const emitter = mitt() + + // Create collection with mutation capability + const collection = new Collection({ + id: `order-by-test`, + sync: { + sync: ({ begin, write, commit }) => { + emitter.on(`sync`, (changes) => { + begin() + ;(changes as Array).forEach((change) => { + write({ + key: change.key, + type: change.type, + value: change.changes as Person, + }) + }) + commit() + }) + }, + }, + }) + + // Sync from initial state + emitter.emit( + `sync`, + initialPersons.map((person) => ({ + key: person.id, + type: `insert`, + changes: person, + })) + ) + + // Test ascending order by age + const ascendingQuery = queryBuilder() + .from({ collection }) + .keyBy(`@id`) + .orderBy(`@age`) + .select(`@id`, `@name`, `@age`) + + const compiledAscendingQuery = compileQuery(ascendingQuery) + compiledAscendingQuery.start() + + const ascendingResult = compiledAscendingQuery.results + + await waitForChanges() + + // Verify ascending order + const ascendingArray = Array.from(ascendingResult.toArray) + expect(ascendingArray).toEqual([ + { id: `2`, name: `Jane Doe`, age: 25, _orderByIndex: 0 }, + { id: `1`, name: `John Doe`, age: 30, _orderByIndex: 1 }, + { id: `3`, name: `John Smith`, age: 35, _orderByIndex: 2 }, + ]) + + // Test descending order by age + const descendingQuery = queryBuilder() + .from({ collection }) + .keyBy(`@id`) + .orderBy({ "@age": `desc` }) + .select(`@id`, `@name`, `@age`) + + const compiledDescendingQuery = compileQuery(descendingQuery) + compiledDescendingQuery.start() + + const descendingResult = compiledDescendingQuery.results + + await waitForChanges() + + // Verify descending order + const descendingArray = Array.from(descendingResult.toArray) + expect(descendingArray).toEqual([ + { id: `3`, name: `John Smith`, age: 35, _orderByIndex: 0 }, + { id: `1`, name: `John Doe`, age: 30, _orderByIndex: 1 }, + { id: `2`, name: `Jane Doe`, age: 25, _orderByIndex: 2 }, + ]) + + // Test multiple order by fields + const multiOrderQuery = queryBuilder() + .from({ collection }) + .keyBy(`@id`) + .orderBy([`@isActive`, { "@age": `asc` }]) + .select(`@id`, `@name`, `@age`, `@isActive`) + + const compiledMultiOrderQuery = compileQuery(multiOrderQuery) + compiledMultiOrderQuery.start() + + const multiOrderResult = compiledMultiOrderQuery.results + + await waitForChanges() + + // Verify multiple field ordering + const multiOrderArray = Array.from(multiOrderResult.toArray) + expect(multiOrderArray).toEqual([ + { + id: `3`, + name: `John Smith`, + age: 35, + isActive: false, + _orderByIndex: 0, + }, + { id: `2`, name: `Jane Doe`, age: 25, isActive: true, _orderByIndex: 1 }, + { id: `1`, name: `John Doe`, age: 30, isActive: true, _orderByIndex: 2 }, + ]) + }) + + it(`should maintain correct ordering when items are added, updated, or deleted`, async () => { + const emitter = mitt() + + // Create collection with mutation capability + const collection = new Collection({ + id: `order-update-test`, + sync: { + sync: ({ begin, write, commit }) => { + emitter.on(`sync`, (changes) => { + begin() + ;(changes as Array).forEach((change) => { + write({ + key: change.key, + type: change.type, + value: change.changes as Person, + }) + }) + commit() + }) + }, + }, + }) + + // Sync from initial state + emitter.emit( + `sync`, + initialPersons.map((person) => ({ + key: person.id, + type: `insert`, + changes: person, + })) + ) + + // Create a query that orders by age in ascending order + const query = queryBuilder() + .from({ collection }) + .keyBy(`@id`) + .orderBy(`@age`) + .select(`@id`, `@name`, `@age`) + + const compiledQuery = compileQuery(query) + compiledQuery.start() + + await waitForChanges() + + // Verify initial ordering + let currentOrder = Array.from(compiledQuery.results.toArray) + expect(currentOrder).toEqual([ + { id: `2`, name: `Jane Doe`, age: 25, _orderByIndex: 0 }, + { id: `1`, name: `John Doe`, age: 30, _orderByIndex: 1 }, + { id: `3`, name: `John Smith`, age: 35, _orderByIndex: 2 }, + ]) + + // Add a new person with the youngest age + emitter.emit(`sync`, [ + { + key: `4`, + type: `insert`, + changes: { + id: `4`, + name: `Alice Young`, + age: 22, + email: `alice.young@example.com`, + isActive: true, + }, + }, + ]) + + await waitForChanges() + + // Verify order is updated with the new person at the beginning + currentOrder = Array.from(compiledQuery.results.toArray) + expect(currentOrder).toEqual([ + { id: `4`, name: `Alice Young`, age: 22, _orderByIndex: 0 }, + { id: `2`, name: `Jane Doe`, age: 25, _orderByIndex: 1 }, + { id: `1`, name: `John Doe`, age: 30, _orderByIndex: 2 }, + { id: `3`, name: `John Smith`, age: 35, _orderByIndex: 3 }, + ]) + + // Update a person's age to move them in the ordering + emitter.emit(`sync`, [ + { + key: `1`, + type: `update`, + changes: { + age: 40, // Update John Doe to be the oldest + }, + }, + ]) + + await waitForChanges() + + // Verify order is updated with John Doe now at the end + currentOrder = Array.from(compiledQuery.results.toArray) + expect(currentOrder).toEqual([ + { id: `4`, name: `Alice Young`, age: 22, _orderByIndex: 0 }, + { id: `2`, name: `Jane Doe`, age: 25, _orderByIndex: 1 }, + { id: `3`, name: `John Smith`, age: 35, _orderByIndex: 2 }, + { id: `1`, name: `John Doe`, age: 40, _orderByIndex: 3 }, + ]) + + // Delete a person in the middle of the ordering + emitter.emit(`sync`, [ + { + key: `3`, + type: `delete`, + }, + ]) + + await waitForChanges() + + // Verify order is updated with John Smith removed + currentOrder = Array.from(compiledQuery.results.toArray) + expect(currentOrder).toEqual([ + { id: `4`, name: `Alice Young`, age: 22, _orderByIndex: 0 }, + { id: `2`, name: `Jane Doe`, age: 25, _orderByIndex: 1 }, + { id: `1`, name: `John Doe`, age: 40, _orderByIndex: 2 }, + ]) + }) }) -async function waitForChanges(ms = 100) { +async function waitForChanges(ms = 0) { await new Promise((resolve) => setTimeout(resolve, ms)) } diff --git a/packages/react-optimistic/src/useLiveQuery.ts b/packages/react-optimistic/src/useLiveQuery.ts index cc05510..76e9a5d 100644 --- a/packages/react-optimistic/src/useLiveQuery.ts +++ b/packages/react-optimistic/src/useLiveQuery.ts @@ -32,7 +32,7 @@ export function useLiveQuery< }, [...deps, restart]) const state = useStore(compiledQuery.results.derivedState) - let data: Array> | undefined + const data = useStore(compiledQuery.results.derivedArray) // Clean up on unmount useEffect(() => { @@ -49,11 +49,6 @@ export function useLiveQuery< return { state, - get data() { - if (!data) { - data = Array.from(state.values()) - } - return data - }, + data, } } diff --git a/packages/react-optimistic/tests/useLiveQuery.test.tsx b/packages/react-optimistic/tests/useLiveQuery.test.tsx index 2c2a305..1f3ad58 100644 --- a/packages/react-optimistic/tests/useLiveQuery.test.tsx +++ b/packages/react-optimistic/tests/useLiveQuery.test.tsx @@ -81,14 +81,13 @@ describe(`Query Collections`, () => { sync: { sync: ({ begin, write, commit }) => { // Listen for sync events - // @ts-expect-error don't trust Mitt's typing and this works. - emitter.on(`sync`, (changes: Array>) => { + emitter.on(`*`, (_, changes) => { begin() - changes.forEach((change) => { + ;(changes as Array).forEach((change) => { write({ key: change.key, type: change.type, - value: change.changes, + value: change.changes as Person, }) }) commit() @@ -116,17 +115,20 @@ describe(`Query Collections`, () => { .where(`@age`, `>`, 30) .keyBy(`@id`) .select(`@id`, `@name`) + .orderBy({ "@id": `asc` }) ) }) expect(result.current.state.size).toBe(1) expect(result.current.state.get(`3`)).toEqual({ + _orderByIndex: 0, id: `3`, name: `John Smith`, }) expect(result.current.data.length).toBe(1) expect(result.current.data[0]).toEqual({ + _orderByIndex: 0, id: `3`, name: `John Smith`, }) @@ -152,20 +154,24 @@ describe(`Query Collections`, () => { expect(result.current.state.size).toBe(2) expect(result.current.state.get(`3`)).toEqual({ + _orderByIndex: 0, id: `3`, name: `John Smith`, }) expect(result.current.state.get(`4`)).toEqual({ + _orderByIndex: 1, id: `4`, name: `Kyle Doe`, }) expect(result.current.data.length).toBe(2) expect(result.current.data).toContainEqual({ + _orderByIndex: 0, id: `3`, name: `John Smith`, }) expect(result.current.data).toContainEqual({ + _orderByIndex: 1, id: `4`, name: `Kyle Doe`, }) @@ -187,12 +193,14 @@ describe(`Query Collections`, () => { expect(result.current.state.size).toBe(2) expect(result.current.state.get(`4`)).toEqual({ + _orderByIndex: 1, id: `4`, name: `Kyle Doe 2`, }) expect(result.current.data.length).toBe(2) expect(result.current.data).toContainEqual({ + _orderByIndex: 1, id: `4`, name: `Kyle Doe 2`, }) @@ -214,6 +222,7 @@ describe(`Query Collections`, () => { expect(result.current.data.length).toBe(1) expect(result.current.data).toContainEqual({ + _orderByIndex: 0, id: `3`, name: `John Smith`, }) @@ -227,22 +236,17 @@ describe(`Query Collections`, () => { id: `person-collection-test`, sync: { sync: ({ begin, write, commit }) => { - // @ts-expect-error Mitt typing doesn't match our usage - emitter.on( - `sync-person`, - // @ts-expect-error Mitt typing doesn't match our usage - (changes: Array>) => { - begin() - changes.forEach((change) => { - write({ - key: change.key, - type: change.type, - value: change.changes, - }) + emitter.on(`sync-person`, (changes) => { + begin() + ;(changes as Array).forEach((change) => { + write({ + key: change.key, + type: change.type, + value: change.changes as Person, }) - commit() - } - ) + }) + commit() + }) }, }, }) @@ -252,24 +256,19 @@ describe(`Query Collections`, () => { id: `issue-collection-test`, sync: { sync: ({ begin, write, commit }) => { - // @ts-expect-error Mitt typing doesn't match our usage - emitter.on(`sync-issue`, (changes: Array>) => { + emitter.on(`sync-issue`, (changes) => { begin() - changes.forEach((change) => { + ;(changes as Array).forEach((change) => { write({ key: change.key, type: change.type, - value: change.changes, + value: change.changes as Issue, }) }) commit() }) }, }, - mutationFn: async ({ transaction }) => { - emitter.emit(`sync-issue`, transaction.mutations) - return Promise.resolve() - }, }) // Sync initial person data @@ -405,14 +404,13 @@ describe(`Query Collections`, () => { sync: { sync: ({ begin, write, commit }) => { // Listen for sync events - // @ts-expect-error don't trust Mitt's typing and this works. - emitter.on(`sync`, (changes: Array>) => { + emitter.on(`sync`, (changes) => { begin() - changes.forEach((change) => { + ;(changes as Array).forEach((change) => { write({ key: change.key, type: change.type, - value: change.changes, + value: change.changes as Person, }) }) commit() @@ -500,14 +498,13 @@ describe(`Query Collections`, () => { id: `stop-query-test`, sync: { sync: ({ begin, write, commit }) => { - // @ts-expect-error Mitt typing doesn't match our usage - emitter.on(`sync`, (changes: Array>) => { + emitter.on(`sync`, (changes) => { begin() - changes.forEach((change) => { + ;(changes as Array).forEach((change) => { write({ key: change.key, type: change.type, - value: change.changes, + value: change.changes as Person, }) }) commit() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ac60652..798c656 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -222,8 +222,8 @@ importers: packages/optimistic: dependencies: '@electric-sql/d2ts': - specifier: ^0.1.4 - version: 0.1.4(@electric-sql/client@1.0.0) + specifier: ^0.1.5 + version: 0.1.5(@electric-sql/client@1.0.0) '@standard-schema/spec': specifier: ^1.0.0 version: 1.0.0 @@ -483,8 +483,8 @@ packages: '@electric-sql/client@1.0.0': resolution: {integrity: sha512-kGiVbBIlMqc/CeJpWZuLjxNkm0836NWxeMtIWH2w5IUK8pUL13hyxg3ZkR7+FlTGhpKuZRiCP5nPOH9D6wbhPw==} - '@electric-sql/d2ts@0.1.4': - resolution: {integrity: sha512-Dv16cDexD1RzrNvIpZPdLF1xVuoe3+j/UjoO84Y3n939UH4sIEF9o8SCVyIo2ngOKMoVXajb2mBI2Ko7Sss8Tw==} + '@electric-sql/d2ts@0.1.5': + resolution: {integrity: sha512-Rmhlmp5G7P229Ul1LLxmnTgY51DIFEw4YJeouyZ9Dg+4nGbOvNAFx2tnb+ghGH77Cd4oMjxqB+94+YasPdu9nA==} peerDependencies: '@electric-sql/client': '>=1.0.0-beta.4' better-sqlite3: ^11.7.0 @@ -5124,7 +5124,7 @@ snapshots: optionalDependencies: '@rollup/rollup-darwin-arm64': 4.36.0 - '@electric-sql/d2ts@0.1.4(@electric-sql/client@1.0.0)': + '@electric-sql/d2ts@0.1.5(@electric-sql/client@1.0.0)': dependencies: fractional-indexing: 3.2.0 optionalDependencies: