🐛 fix: Preserve numeric string keys as objects instead of arrays#164
🐛 fix: Preserve numeric string keys as objects instead of arrays#164canisminor1990 merged 1 commit intolobehub:masterfrom
Conversation
Fixed issue where objects with numeric string keys (e.g., {"0": {...}, "1": {...}})
were being converted to arrays during i18n translation processing.
The root cause was lodash set() treating numeric string keys as array indices.
Implemented custom setByPath() function that always treats keys as object properties.
Related: Comfy-Org/ComfyUI_frontend#8718
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Reviewer's GuideReplaces lodash’s generic set() usage in i18n JSON diffing with a custom path setter that always treats keys as object properties, ensuring numeric string keys are preserved as objects rather than coerced to array indices, and adds focused tests for this behavior. Class diagram for setByPath utility and diffJson moduleclassDiagram
class DiffJsonUtils {
+diff(oldLocale: LocaleObj, newLocale: LocaleObj, keyStyle: KeyStyle, config: I18nConfig) LocaleObj
+resolveDiffPath(entry: LocaleObj, path: DiffPath, keyStyle: KeyStyle) DiffPath
-hasOwnKey(obj: LocaleObj, key: string) boolean
}
class SetByPathUtils {
+setByPath(obj: any, path: Array~string_or_number (string or number)~, value: any) void
}
class LocaleObj
class I18nConfig
class KeyStyle
class DiffPath {
+path: string_or_Array_of_number_or_string (string | Array<number|string>)
}
DiffJsonUtils --> SetByPathUtils : uses
DiffJsonUtils ..> LocaleObj
DiffJsonUtils ..> I18nConfig
DiffJsonUtils ..> KeyStyle
DiffJsonUtils ..> DiffPath
Flow diagram for diff JSON processing using setByPathflowchart TD
Caller["Call diff(oldLocale, newLocale, keyStyle, config)"] --> DiffFn
subgraph DiffModule["diffJson.ts"]
DiffFn["Function diff"]
JustDiff["justDiff diff"]
ResolvePath["Function resolveDiffPath"]
ExtraObj["extra locale object"]
end
subgraph SetByPathModule["setByPath.ts"]
SetByPath["Function setByPath(obj, pathArray, value)"]
end
DiffFn --> JustDiff
JustDiff -->|"operations: add, remove, update"| DiffFn
DiffFn -->|"for each add item"| ResolvePath
ResolvePath -->|"DiffPath or string"| DiffFn
DiffFn -->|"normalize to pathArray"| PathArray["Path as array of keys"]
PathArray --> SetByPath
SetByPath -->|"walks path, creates intermediate objects"| ExtraObj
SetByPath -->|"sets finalKey as string"| ExtraObj
ExtraObj -->|"returned as extra"| Caller
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
👍 @snomiao Thank you for raising your pull request and contributing to our Community |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- By wrapping a string
pathfromresolveDiffPathin an array ([path]),setByPathnow treats it as a literal key instead of a dotted path (unlikelodash.set); if any callers rely on dot-notation semantics fromresolveDiffPath, this will change behavior and may need to be addressed either inresolveDiffPathorsetByPath. setByPathcurrently assumesobjis a non-null object and uses very looseanytypes; consider tightening the types (e.g.Record<string, unknown>) and early-returning or throwing whenobjisnull/non-object to avoid silent failures or runtime errors when misused.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- By wrapping a string `path` from `resolveDiffPath` in an array (`[path]`), `setByPath` now treats it as a literal key instead of a dotted path (unlike `lodash.set`); if any callers rely on dot-notation semantics from `resolveDiffPath`, this will change behavior and may need to be addressed either in `resolveDiffPath` or `setByPath`.
- `setByPath` currently assumes `obj` is a non-null object and uses very loose `any` types; consider tightening the types (e.g. `Record<string, unknown>`) and early-returning or throwing when `obj` is `null`/non-object to avoid silent failures or runtime errors when misused.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Fixes i18n JSON diff processing so objects with numeric string keys (e.g. { "0": ... }) aren’t accidentally turned into arrays when building translation “extra” chunks.
Changes:
- Added a custom
setByPathutility to replacelodash.setand preserve numeric-string keys as object properties. - Updated
diffJsonto usesetByPathwhen building the “added keys” object. - Added unit tests covering
setByPathbehavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/lobe-i18n/src/utils/setByPath.ts |
New path-setter utility used to avoid lodash numeric-key-to-array behavior. |
packages/lobe-i18n/src/utils/setByPath.test.ts |
Unit tests for the new setter, including numeric key scenarios. |
packages/lobe-i18n/src/utils/diffJson.ts |
Swaps lodash.set for setByPath when constructing extra additions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Create intermediate object if it doesn't exist or is not an object | ||
| if (!current[key] || typeof current[key] !== 'object') { | ||
| current[key] = {}; |
There was a problem hiding this comment.
setByPath currently stringifies every path segment and always creates {} for missing intermediates. This prevents creating real arrays when the path contains a numeric index (e.g. ['array', 3] from just-diff), so added array elements will be represented as objects with '3' keys instead of sparse arrays (behavior change vs previous lodash.set). Consider preserving arrays by creating [] when the next path segment is a number (array index), while still treating numeric strings (e.g. '0') as object keys to fix the reported issue.
| // 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' ? [] : {}; |
| 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); | ||
| }); |
There was a problem hiding this comment.
The test should handle number type in path asserts that numeric number segments (0/1) are treated as object keys. In production, array diffs from just-diff use number indices (see existing diffJson tests), so encoding number segments as object properties makes it impossible for setByPath to preserve real arrays. Recommend adjusting this test (and adding a dedicated array case) so number path segments can represent array indices, while numeric string segments remain object keys.
| for (const item of add) { | ||
| const path = resolveDiffPath(entry, item.path as DiffPath, keyStyle); | ||
| set(extra, path, item.value); | ||
| // Use custom setByPath to preserve numeric string keys as object keys | ||
| const pathArray = Array.isArray(path) ? path : [path]; | ||
| setByPath(extra, pathArray, item.value); |
There was a problem hiding this comment.
diffJson.diff now relies on setByPath to build extra for 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 in result.entry. Adding/adjusting tests in diffJson.test.ts to assert result.entry for these cases would help prevent regressions in the translation chunk building pipeline.
|
❤️ Great PR @snomiao ❤️ The growth of project is inseparable from user feedback and contribution, thanks for your contribution! |
### [Version 1.26.1](https://github.com/lobehub/lobe-cli-toolbox/compare/@lobehub/[email protected]...@lobehub/[email protected]) <sup>Released on **2026-02-15**</sup> #### 🐛 Bug Fixes - **misc**: Preserve numeric string keys as objects instead of arrays. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's fixed * **misc**: Preserve numeric string keys as objects instead of arrays, closes [#164](#164) [Comfy-Org/ComfyUI_frontend#8718](Comfy-Org/ComfyUI_frontend#8718) ([9645ed8](9645ed8)) </details> <div align="right"> [](#readme-top) </div>
|
🎉 This PR is included in version 1.26.1 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
3 participants
Summary
Fixes the issue where objects with numeric string keys (e.g.,
{"0": {...}, "1": {...}}) were being converted to arrays during i18n translation processing.Root Cause
The
lodash.set()function interprets numeric string keys as array indices, which caused:{"0": "value1", "1": "value2"}to become arrays["value1", "value2"]Solution
Implemented a custom
setByPath()function that:Changes
packages/lobe-i18n/src/utils/setByPath.ts- Custom path setter that preserves numeric keyspackages/lobe-i18n/src/utils/setByPath.test.ts- Comprehensive testspackages/lobe-i18n/src/utils/diffJson.ts- Use custom setter instead of lodashTest Plan
setByPath()(11/11)Related
Before/After
Before:
{ "nodeDefs": ["node1", "node2"] // ❌ Array }After:
{ "nodeDefs": { // ✅ Object "0": "node1", "1": "node2" } }🤖 Generated with Claude Code
Summary by Sourcery
Preserve numeric string keys as object properties during i18n diff processing instead of having them coerced into arrays.
Bug Fixes:
Enhancements:
Tests: