Skip to content

Commit 80b5ea0

Browse files
sergeyshmakovsergeyshmakov
committed
fix: code review fixes
1 parent c714b0f commit 80b5ea0

4 files changed

Lines changed: 263 additions & 206 deletions

File tree

‎src/path.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { PATH_SEGMENTS } from "./constants.js";
22
import { PathImpl, setTemplatePathCtor } from "./impl/path-impl.js";
33
import { TemplatePathImpl } from "./impl/template-path-impl.js";
44
import type { BasePath, Path, PathExpression, Segment } from "./types.js";
5-
import { createPathProxy } from "./utils.js";
5+
import { createPathProxy, isCanonicalArrayIndex } from "./utils.js";
66

77
setTemplatePathCtor(TemplatePathImpl);
88

@@ -86,7 +86,9 @@ export function unsafePath<T>(raw: string): Path<T, unknown, string> {
8686
const segments: Segment[] = raw
8787
? raw
8888
.split(".")
89-
.map((s) => (s === "" ? s : Number.isNaN(Number(s)) ? s : Number(s)))
89+
.map((s) =>
90+
s === "" ? s : isCanonicalArrayIndex(s) ? Number(s) : s,
91+
)
9092
: [];
9193
return new PathImpl<T, unknown, string>(segments);
9294
}

‎src/tests/data-access.spec.ts‎

Lines changed: 173 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -7,152 +7,180 @@ import { describe, expect, it } from "vitest";
77
import { path } from "../index.js";
88

99
interface Shop {
10-
products: Array<{
11-
id: string;
12-
photos: Array<{ id: string; url: string }>;
13-
}>;
10+
products: Array<{
11+
id: string;
12+
photos: Array<{ id: string; url: string }>;
13+
}>;
1414
}
1515

1616
describe("Data access", () => {
17-
describe(".get()", () => {
18-
it("retrieves nested properties", () => {
19-
const p = path((x: { a: { b: { c: string } } }) => x.a.b.c);
20-
const data = { a: { b: { c: "hello" } } };
21-
expect(p.get(data)).toBe("hello");
22-
});
23-
24-
it("handles arrays", () => {
25-
const p = path((x: { items: string[] }) => x.items[1]);
26-
const data = { items: ["a", "b", "c"] };
27-
expect(p.get(data)).toBe("b");
28-
});
29-
30-
it("returns undefined when intermediate objects are missing", () => {
31-
const p = path<{ a?: { b?: { c: string } } }>(
32-
(x) => (x as { a: { b: { c: string } } }).a.b.c,
33-
);
34-
const data = { a: undefined };
35-
expect(p.get(data)).toBeUndefined();
36-
});
37-
38-
it("immutability: does not mutate original path", () => {
39-
const p = path((x: { a: { b: { c: string } } }) => x.a.b.c);
40-
const data = { a: { b: { c: "hello" } } };
41-
p.get(data);
42-
expect(p.segments).toEqual(["a", "b", "c"]);
43-
});
44-
});
45-
46-
describe(".set()", () => {
47-
it("immutably updates nested property", () => {
48-
const p = path((x: { a: { b: number } }) => x.a.b);
49-
const data = { a: { b: 1 } };
50-
const updated = p.set(data, 42);
51-
expect(updated.a.b).toBe(42);
52-
expect(data.a.b).toBe(1);
53-
});
54-
55-
it("immutability: does not mutate original path", () => {
56-
const p = path((x: { a: { b: number } }) => x.a.b);
57-
const data = { a: { b: 1 } };
58-
p.set(data, 42);
59-
expect(p.segments).toEqual(["a", "b"]);
60-
});
61-
62-
describe("deep immutability and structural sharing", () => {
63-
const getFixture = (): Shop => ({
64-
products: [
65-
{
66-
id: "p1",
67-
photos: [
68-
{ id: "ph1_1", url: "url1_1" },
69-
{ id: "ph1_2", url: "url1_2" },
70-
],
71-
},
72-
{
73-
id: "p2",
74-
photos: [{ id: "ph2_1", url: "url2_1" }],
75-
},
76-
],
77-
});
78-
79-
it("creates new references only along the updated path", () => {
80-
const data = getFixture();
81-
const p = path((x: Shop) => x.products[0].photos[1].url);
82-
83-
const updated = p.set(data, "new_url");
84-
85-
// Value is updated
86-
expect(updated.products[0].photos[1].url).toBe("new_url");
87-
// Original is unchanged
88-
expect(data.products[0].photos[1].url).toBe("url1_2");
89-
90-
// Root reference changed
91-
expect(updated).not.toBe(data);
92-
// Array reference changed
93-
expect(updated.products).not.toBe(data.products);
94-
// Object reference changed
95-
expect(updated.products[0]).not.toBe(data.products[0]);
96-
// Inner array reference changed
97-
expect(updated.products[0].photos).not.toBe(data.products[0].photos);
98-
// Inner object reference changed
99-
expect(updated.products[0].photos[1]).not.toBe(
100-
data.products[0].photos[1],
101-
);
102-
});
103-
104-
it("preserves original references for unchanged branches (structural sharing)", () => {
105-
const data = getFixture();
106-
const p = path((x: Shop) => x.products[0].photos[1].url);
107-
108-
const updated = p.set(data, "new_url");
109-
110-
// Unchanged sibling in the products array is reused
111-
expect(updated.products[1]).toBe(data.products[1]);
112-
// Unchanged primitive/reference in the modified product object is reused
113-
expect(updated.products[0].id).toBe(data.products[0].id);
114-
// Unchanged sibling in the modified photos array is reused
115-
expect(updated.products[0].photos[0]).toBe(data.products[0].photos[0]);
116-
});
117-
});
118-
});
119-
120-
describe("unexpected cases", () => {
121-
it(".get(null) and .get(undefined) gracefully returns undefined", () => {
122-
const p = path((x: { a: number }) => x.a);
123-
expect(p.get(null as any)).toBeUndefined();
124-
expect(p.get(undefined as any)).toBeUndefined();
125-
});
126-
127-
it(".get() and .set() handle unexpected structures safely", () => {
128-
const p = path((x: { a: { b: number } }) => x.a.b);
129-
// Trying to traverse into a string
130-
expect(p.get({ a: "string" } as any)).toBeUndefined();
131-
132-
// Trying to set into a string replaces the string with an object/array at that level?
133-
// Or at least it doesn't crash the host program unexpectedly.
134-
expect(() => p.set({ a: "string" } as any, 42)).not.toThrow();
135-
});
136-
137-
it("Array .set() operations with out-of-bounds indices or negative indices", () => {
138-
const p = path((x: { items: string[] }) => x.items[5]);
139-
const data = { items: ["a"] };
140-
const result = p.set(data, "f");
141-
expect(result.items[5]).toBe("f");
142-
expect(result.items.length).toBeGreaterThan(1);
143-
144-
// Negative index behavior checks
145-
const pNeg = path((x: { items: string[] }) => x.items[-1]);
146-
expect(() => pNeg.set(data, "z")).not.toThrow();
147-
});
148-
});
149-
150-
describe("typing incorrect cases", () => {
151-
it("rejects .set() with a wrong type", () => {
152-
const p = path((x: { a: number }) => x.a);
153-
const data = { a: 1 };
154-
// @ts-expect-error
155-
p.set(data, "wrong-type");
156-
});
157-
});
17+
describe(".get()", () => {
18+
it("retrieves nested properties", () => {
19+
const p = path((x: { a: { b: { c: string } } }) => x.a.b.c);
20+
const data = { a: { b: { c: "hello" } } };
21+
expect(p.get(data)).toBe("hello");
22+
});
23+
24+
it("handles arrays", () => {
25+
const p = path((x: { items: string[] }) => x.items[1]);
26+
const data = { items: ["a", "b", "c"] };
27+
expect(p.get(data)).toBe("b");
28+
});
29+
30+
it("returns undefined when intermediate objects are missing", () => {
31+
const p = path<{ a?: { b?: { c: string } } }>(
32+
(x) => (x as { a: { b: { c: string } } }).a.b.c,
33+
);
34+
const data = { a: undefined };
35+
expect(p.get(data)).toBeUndefined();
36+
});
37+
38+
it("immutability: does not mutate original path", () => {
39+
const p = path((x: { a: { b: { c: string } } }) => x.a.b.c);
40+
const data = { a: { b: { c: "hello" } } };
41+
p.get(data);
42+
expect(p.segments).toEqual(["a", "b", "c"]);
43+
});
44+
});
45+
46+
describe(".set()", () => {
47+
it("immutably updates nested property", () => {
48+
const p = path((x: { a: { b: number } }) => x.a.b);
49+
const data = { a: { b: 1 } };
50+
const updated = p.set(data, 42);
51+
expect(updated.a.b).toBe(42);
52+
expect(data.a.b).toBe(1);
53+
});
54+
55+
it("immutability: does not mutate original path", () => {
56+
const p = path((x: { a: { b: number } }) => x.a.b);
57+
const data = { a: { b: 1 } };
58+
p.set(data, 42);
59+
expect(p.segments).toEqual(["a", "b"]);
60+
});
61+
62+
describe("deep immutability and structural sharing", () => {
63+
const getFixture = (): Shop => ({
64+
products: [
65+
{
66+
id: "p1",
67+
photos: [
68+
{ id: "ph1_1", url: "url1_1" },
69+
{ id: "ph1_2", url: "url1_2" },
70+
],
71+
},
72+
{
73+
id: "p2",
74+
photos: [{ id: "ph2_1", url: "url2_1" }],
75+
},
76+
],
77+
});
78+
79+
it("creates new references only along the updated path", () => {
80+
const data = getFixture();
81+
const p = path((x: Shop) => x.products[0].photos[1].url);
82+
83+
const updated = p.set(data, "new_url");
84+
85+
// Value is updated
86+
expect(updated.products[0].photos[1].url).toBe("new_url");
87+
// Original is unchanged
88+
expect(data.products[0].photos[1].url).toBe("url1_2");
89+
90+
// Root reference changed
91+
expect(updated).not.toBe(data);
92+
// Array reference changed
93+
expect(updated.products).not.toBe(data.products);
94+
// Object reference changed
95+
expect(updated.products[0]).not.toBe(data.products[0]);
96+
// Inner array reference changed
97+
expect(updated.products[0].photos).not.toBe(
98+
data.products[0].photos,
99+
);
100+
// Inner object reference changed
101+
expect(updated.products[0].photos[1]).not.toBe(
102+
data.products[0].photos[1],
103+
);
104+
});
105+
106+
it("preserves original references for unchanged branches (structural sharing)", () => {
107+
const data = getFixture();
108+
const p = path((x: Shop) => x.products[0].photos[1].url);
109+
110+
const updated = p.set(data, "new_url");
111+
112+
// Unchanged sibling in the products array is reused
113+
expect(updated.products[1]).toBe(data.products[1]);
114+
// Unchanged primitive/reference in the modified product object is reused
115+
expect(updated.products[0].id).toBe(data.products[0].id);
116+
// Unchanged sibling in the modified photos array is reused
117+
expect(updated.products[0].photos[0]).toBe(
118+
data.products[0].photos[0],
119+
);
120+
});
121+
});
122+
});
123+
124+
describe("unexpected cases", () => {
125+
it(".get(null) and .get(undefined) gracefully returns undefined", () => {
126+
const p = path((x: { a: number }) => x.a);
127+
expect(p.get(null as any)).toBeUndefined();
128+
expect(p.get(undefined as any)).toBeUndefined();
129+
});
130+
131+
it(".get() and .set() handle unexpected structures safely", () => {
132+
const p = path((x: { a: { b: number } }) => x.a.b);
133+
// Trying to traverse into a string
134+
expect(p.get({ a: "string" } as any)).toBeUndefined();
135+
136+
// Trying to set into a string replaces the string with an object/array at that level?
137+
// Or at least it doesn't crash the host program unexpectedly.
138+
expect(() => p.set({ a: "string" } as any, 42)).not.toThrow();
139+
});
140+
141+
it("records 'then' as a normal path segment", () => {
142+
const p = path((x: { then: { value: string } }) => x.then.value);
143+
// biome-ignore lint/suspicious/noThenProperty: testing that 'then' is recorded as a normal path segment
144+
const data = { then: { value: "resolved" } };
145+
expect(p.get(data)).toBe("resolved");
146+
expect(p.segments).toEqual(["then", "value"]);
147+
});
148+
149+
it("preserves numeric-looking object keys as strings (e.g. '01', '1e3')", () => {
150+
// "01" and "1" are distinct keys in JS; "1e3" and "1000" are distinct
151+
const p01 = path((x: { "01": string }) => x["01"]);
152+
const p1e3 = path((x: { "1e3": number }) => x["1e3"]);
153+
const data = {
154+
"01": "value-01",
155+
1: "value-1",
156+
"1e3": 1000,
157+
1000: 999,
158+
};
159+
expect(p01.get(data)).toBe("value-01");
160+
expect(p1e3.get(data)).toBe(1000);
161+
expect(p01.segments).toEqual(["01"]);
162+
expect(p1e3.segments).toEqual(["1e3"]);
163+
});
164+
165+
it("Array .set() operations with out-of-bounds indices or negative indices", () => {
166+
const p = path((x: { items: string[] }) => x.items[5]);
167+
const data = { items: ["a"] };
168+
const result = p.set(data, "f");
169+
expect(result.items[5]).toBe("f");
170+
expect(result.items.length).toBeGreaterThan(1);
171+
172+
// Negative index behavior checks
173+
const pNeg = path((x: { items: string[] }) => x.items[-1]);
174+
expect(() => pNeg.set(data, "z")).not.toThrow();
175+
});
176+
});
177+
178+
describe("typing incorrect cases", () => {
179+
it("rejects .set() with a wrong type", () => {
180+
const p = path((x: { a: number }) => x.a);
181+
const data = { a: 1 };
182+
// @ts-expect-error
183+
p.set(data, "wrong-type");
184+
});
185+
});
158186
});

‎src/tests/unsafe.spec.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ describe("unsafePath()", () => {
2626
expect(p.segments).toEqual(["items", 0]);
2727
});
2828

29+
it("preserves numeric-looking object keys as strings", () => {
30+
const p = unsafePath<object>("data.01");
31+
expect(p.segments).toEqual(["data", "01"]);
32+
const p2 = unsafePath<object>("a.1e3.b");
33+
expect(p2.segments).toEqual(["a", "1e3", "b"]);
34+
});
35+
2936
it("handles malformed strings with best-effort parse", () => {
3037
const p = unsafePath<object>("a..b");
3138
expect(p.segments).toContain("");

0 commit comments

Comments
 (0)