Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/loose-flies-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@tanstack/solid-db': patch
'@tanstack/db': patch
---

Fixed a bug where cloning non enumerable properties causes solid store to get stuck in a non current state

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Polish release note wording for clarity.

Use hyphenation and product casing for readability in changelog text.

Suggested edit
-Fixed a bug where cloning non enumerable properties causes solid store to get stuck in a non current state
+Fixed a bug where cloning non-enumerable properties causes the Solid store to get stuck in a non-current state.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Fixed a bug where cloning non enumerable properties causes solid store to get stuck in a non current state
Fixed a bug where cloning non-enumerable properties causes the Solid store to get stuck in a non-current state.
🧰 Tools
🪛 LanguageTool

[grammar] ~6-~6: Use a hyphen to join words.
Context: ...patch --- Fixed a bug where cloning non enumerable properties causes solid store...

(QB_NEW_EN_HYPHEN)


[grammar] ~6-~6: Use a hyphen to join words.
Context: ...causes solid store to get stuck in a non current state

(QB_NEW_EN_HYPHEN)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.changeset/loose-flies-sell.md at line 6, The release note in the changeset
file needs improved formatting for clarity. Apply hyphenation to compound
adjectives by adding hyphens to "non enumerable" and "non current".
Additionally, apply proper product casing to "solid store" to match standard
product naming conventions. These changes will improve the readability and
professionalism of the changelog entry.

Source: Linters/SAST tools

2 changes: 2 additions & 0 deletions packages/db/src/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,8 @@ function deepClone<T extends unknown>(

const symbolProps = Object.getOwnPropertySymbols(obj)
for (const sym of symbolProps) {
if (!Object.prototype.propertyIsEnumerable.call(obj, sym)) continue

clone[sym] = deepClone(
(obj as Record<string | symbol, unknown>)[sym],
visited,
Expand Down
28 changes: 28 additions & 0 deletions packages/db/tests/proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,34 @@ describe(`Proxy Library`, () => {
})
})

it(`should not clone non-enumerable symbol metatdata into nested changes`, () => {
const metadata = Symbol(`metadata`)
const domainSymbol = Symbol(`domain`)

const obj = {
user: {
name: `John`,
age: 30,
[domainSymbol]: `preserved`,
},
}

Object.defineProperty(obj.user, metadata, {
value: `internal`,
enumerable: false,
})

const changes = withChangeTracking(obj, (draft) => {
draft.user.age = 31
})

const changedUser = changes.user as Record<string | symbol, unknown>

expect(changedUser.age).toBe(31)
expect(changedUser[domainSymbol]).toBe(`preserved`)
expect(Object.getOwnPropertySymbols(changedUser)).not.toContain(metadata)
})

it(`should track when properties are deleted from objects`, () => {
const obj: { name: string; age: number; role?: string } = {
name: `John`,
Expand Down
115 changes: 115 additions & 0 deletions packages/solid-db/tests/useLiveQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ type Issue = {
userId: string
}

type DeepNode = {
id: string
size: {
min: number
max: number
}
}

const initialPersons: Array<Person> = [
{
id: `1`,
Expand Down Expand Up @@ -2587,4 +2595,111 @@ describe(`Query Collections`, () => {
expect(rendered.result()).toBeUndefined()
})
})

it('single-row reflects the current value on remount', async () => {
const nodeId = 'node-1'
const collection = createCollection(
mockSyncCollectionOptions<DeepNode>({
id: `remount-single-nested-test`,
getKey: (item) => item.id,
initialData: [
{
id: nodeId,
size: { min: 1, max: 10 },
},
],
}),
)

const updateMax = (max: number) => {
collection.update(nodeId, (draft) => {
draft.size.max = max
})
}

const mount = () =>
render(() => {
const query = useLiveQuery((q) =>
q
.from({ source: collection })
.where(({ source }) => eq(source.id, nodeId))
.findOne(),
)

return <span data-testid="value">{query()?.size.max ?? 'pending'}</span>
})

const first = mount()

await waitFor(() =>
expect(first.getByTestId(`value`).textContent).toBe(`10`),
)

updateMax(777)

await waitFor(() =>
expect(first.getByTestId(`value`).textContent).toBe(`777`),
)

first.unmount()

const second = mount()
await waitFor(() =>
expect(second.getByTestId(`value`).textContent).toBe(`777`),
)
})

it('array reflects the current value on remount', async () => {
const nodeId = 'node-1'
const collection = createCollection(
mockSyncCollectionOptions<DeepNode>({
id: `remount-single-nested-test`,
getKey: (item) => item.id,
initialData: [
{
id: nodeId,
size: { min: 1, max: 10 },
},
],
}),
)

const updateMax = (max: number) => {
collection.update(nodeId, (draft) => {
draft.size.max = max
})
}

const mount = () =>
render(() => {
const query = useLiveQuery((q) =>
q
.from({ source: collection })
.where(({ source }) => eq(source.id, nodeId)),
)

return (
<span data-testid="value">{query()[0]?.size.max ?? 'pending'}</span>
)
})

const first = mount()

await waitFor(() =>
expect(first.getByTestId(`value`).textContent).toBe(`10`),
)

updateMax(777)

await waitFor(() =>
expect(first.getByTestId(`value`).textContent).toBe(`777`),
)

first.unmount()

const second = mount()
await waitFor(() =>
expect(second.getByTestId(`value`).textContent).toBe(`777`),
)
})
})