-
-
Notifications
You must be signed in to change notification settings - Fork 62
🐛 fix: Preserve numeric string keys as objects instead of arrays #164
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,146 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { setByPath } from './setByPath'; | ||
|
|
||
| describe('setByPath', () => { | ||
| it('should set a simple nested value', () => { | ||
| const obj = {}; | ||
| setByPath(obj, ['a', 'b', 'c'], 'value'); | ||
| expect(obj).toEqual({ a: { b: { c: 'value' } } }); | ||
| }); | ||
|
|
||
| it('should preserve numeric string keys as object keys, not array indices', () => { | ||
| const obj = {}; | ||
| setByPath(obj, ['nodeDefs', '0', 'name'], 'First Node'); | ||
| setByPath(obj, ['nodeDefs', '1', 'name'], 'Second Node'); | ||
| setByPath(obj, ['nodeDefs', '2', 'name'], 'Third Node'); | ||
|
|
||
| expect(obj).toEqual({ | ||
| nodeDefs: { | ||
| '0': { name: 'First Node' }, | ||
| '1': { name: 'Second Node' }, | ||
| '2': { name: 'Third Node' }, | ||
| }, | ||
| }); | ||
|
|
||
| // Verify it's an object, not an array | ||
| expect(Array.isArray((obj as any).nodeDefs)).toBe(false); | ||
| }); | ||
|
|
||
| it('should handle numeric keys at the root level', () => { | ||
| const obj = {}; | ||
| setByPath(obj, ['0'], 'value0'); | ||
| setByPath(obj, ['1'], 'value1'); | ||
|
|
||
| expect(obj).toEqual({ | ||
| '0': 'value0', | ||
| '1': 'value1', | ||
| }); | ||
|
|
||
| expect(Array.isArray(obj)).toBe(false); | ||
| }); | ||
|
|
||
| it('should handle mixed numeric and string keys', () => { | ||
| const obj = {}; | ||
| setByPath(obj, ['items', '0', 'id'], 'first'); | ||
| setByPath(obj, ['items', 'abc', 'id'], 'second'); | ||
| setByPath(obj, ['items', '1', 'id'], 'third'); | ||
|
|
||
| expect(obj).toEqual({ | ||
| items: { | ||
| '0': { id: 'first' }, | ||
| '1': { id: 'third' }, | ||
| 'abc': { id: 'second' }, | ||
| }, | ||
| }); | ||
|
|
||
| expect(Array.isArray((obj as any).items)).toBe(false); | ||
| }); | ||
|
|
||
| it('should overwrite existing values', () => { | ||
| const obj = { a: { b: 'old' } }; | ||
| setByPath(obj, ['a', 'b'], 'new'); | ||
| expect(obj).toEqual({ a: { b: 'new' } }); | ||
| }); | ||
|
|
||
| it('should handle single-key paths', () => { | ||
| const obj = {}; | ||
| setByPath(obj, ['key'], 'value'); | ||
| expect(obj).toEqual({ key: 'value' }); | ||
| }); | ||
|
|
||
| it('should handle empty path gracefully', () => { | ||
| const obj = { existing: 'data' }; | ||
| setByPath(obj, [], 'value'); | ||
| // Should not modify the object | ||
| expect(obj).toEqual({ existing: 'data' }); | ||
| }); | ||
|
|
||
| it('should create intermediate objects when path does not exist', () => { | ||
| const obj = {}; | ||
| setByPath(obj, ['a', 'b', 'c', 'd'], 'value'); | ||
| expect(obj).toEqual({ | ||
| a: { | ||
| b: { | ||
| c: { | ||
| d: 'value', | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it('should replace non-object intermediate values with objects', () => { | ||
| const obj: any = { a: 'string' }; | ||
| setByPath(obj, ['a', 'b'], 'value'); | ||
| expect(obj).toEqual({ | ||
| a: { | ||
| b: 'value', | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it('should handle complex nested structures with numeric keys', () => { | ||
| const obj = {}; | ||
| setByPath(obj, ['nodeDefs', '0', 'inputs', '0', 'name'], 'input1'); | ||
| setByPath(obj, ['nodeDefs', '0', 'inputs', '1', 'name'], 'input2'); | ||
| setByPath(obj, ['nodeDefs', '1', 'outputs', '0', 'name'], 'output1'); | ||
|
|
||
| expect(obj).toEqual({ | ||
| nodeDefs: { | ||
| '0': { | ||
| inputs: { | ||
| '0': { name: 'input1' }, | ||
| '1': { name: 'input2' }, | ||
| }, | ||
| }, | ||
| '1': { | ||
| outputs: { | ||
| '0': { name: 'output1' }, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| // Verify all levels are objects, not arrays | ||
| const typed = obj as any; | ||
| expect(Array.isArray(typed.nodeDefs)).toBe(false); | ||
| expect(Array.isArray(typed.nodeDefs['0'].inputs)).toBe(false); | ||
| expect(Array.isArray(typed.nodeDefs['1'].outputs)).toBe(false); | ||
| }); | ||
|
|
||
| it('should handle number type in path (not just numeric strings)', () => { | ||
| const obj = {}; | ||
| setByPath(obj, ['items', 0, 'name'], 'First'); | ||
| setByPath(obj, ['items', 1, 'name'], 'Second'); | ||
|
|
||
| expect(obj).toEqual({ | ||
| items: { | ||
| '0': { name: 'First' }, | ||
| '1': { name: 'Second' }, | ||
| }, | ||
| }); | ||
|
|
||
| expect(Array.isArray((obj as any).items)).toBe(false); | ||
| }); | ||
|
Comment on lines
+132
to
+145
|
||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,32 @@ | ||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * Custom implementation of lodash set() that preserves numeric string keys as object keys | ||||||||||||||||||||||||||||
| * instead of converting them to array indices. | ||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||
| * This fixes the issue where objects like {"0": {...}, "1": {...}} were being converted | ||||||||||||||||||||||||||||
| * to arrays during the i18n translation process. | ||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||
| * @param obj - The object to set the value in | ||||||||||||||||||||||||||||
| * @param path - Array of keys representing the path | ||||||||||||||||||||||||||||
| * @param value - The value to set | ||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| export function setByPath(obj: any, path: Array<string | number>, value: any): void { | ||||||||||||||||||||||||||||
| if (path.length === 0) return; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| let current = obj; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Navigate to the parent of the target key | ||||||||||||||||||||||||||||
| for (let i = 0; i < path.length - 1; i++) { | ||||||||||||||||||||||||||||
| const key = String(path[i]); // Always treat keys as strings | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Create intermediate object if it doesn't exist or is not an object | ||||||||||||||||||||||||||||
| if (!current[key] || typeof current[key] !== 'object') { | ||||||||||||||||||||||||||||
| current[key] = {}; | ||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+23
|
||||||||||||||||||||||||||||
| // Create intermediate object if it doesn't exist or is not an object | |
| if (!current[key] || typeof current[key] !== 'object') { | |
| current[key] = {}; | |
| // Create intermediate container if it doesn't exist or is not an object. | |
| // If the next path segment is a number, we create an array to preserve | |
| // array semantics (e.g. ['items', 3]); otherwise we create an object. | |
| if ( | |
| current[key] === undefined || | |
| current[key] === null || | |
| typeof current[key] !== 'object' | |
| ) { | |
| const nextSegment = path[i + 1]; | |
| current[key] = typeof nextSegment === 'number' ? [] : {}; |
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.
diffJson.diffnow relies onsetByPathto buildextrafor added paths, but there’s no integration coverage here for (1) numeric-string object keys staying objects (the reported bug) and (2) array index paths (e.g.['array', 3]) producing the expected structure inresult.entry. Adding/adjusting tests indiffJson.test.tsto assertresult.entryfor these cases would help prevent regressions in the translation chunk building pipeline.