Skip to content

fix(cli): Fix composite JSON #1108

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.

Sign up for GitHub

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

Merged
fernando-aviles merged 4 commits into from
Mar 14, 2026
Merged

fix(cli): Fix composite JSON #1108

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9925731
fix
brian-lou Mar 14, 2026
65c5408
update tests
brian-lou Mar 14, 2026
9eb0d3d
fix: parsing
fernando-aviles Mar 14, 2026
852a81f
fix: leave missing unchanged
fernando-aviles Mar 14, 2026
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
5 changes: 5 additions & 0 deletions .changeset/smart-bobcats-cheat.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'gt': patch
---

Fix string behavior
6 changes: 6 additions & 0 deletions packages/cli/src/formats/json/extractJson.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ export function extractJson(
continue;
}

// If the source item is a string, use it directly
if (typeof matchingTargetItem.sourceItem === 'string') {
compositeResult[sourceObjectPointer] = matchingTargetItem.sourceItem;
continue;
}

// Extract values at the include paths
const extractedValues = flattenJsonWithStringFilter(
matchingTargetItem.sourceItem,
Expand Down
13 changes: 12 additions & 1 deletion packages/cli/src/formats/json/mergeJson.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,21 @@ export function mergeJson(
sourceObjectOptions,
sourceObjectValue
);
const mutateSourceItemKey = matchingTargetItem.keyParentProperty;

// If the source item is a string, use the translated string directly
if (typeof defaultLocaleSourceItem === 'string') {
const translatedValue =
typeof targetItems === 'string'
? targetItems
: defaultLocaleSourceItem;
sourceObjectValue[mutateSourceItemKey] = translatedValue;
continue;
}
Comment on lines +351 to +357
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 14, 2026

Choose a reason for hiding this comment

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

targetItems replaced before string check

The !targetItems guard on line 331–333 replaces a falsy targetItems with {} before the new string path is reached. An empty-string translation ("") is falsy, so it gets silently overwritten with {}, causing typeof targetItems === 'string' to be false and the code to fall back to defaultLocaleSourceItem. This means an empty translated string in the source map (intentional or otherwise) is silently discarded and replaced with the default-locale value.

let targetItems = targetJson[sourceObjectPointer];
if (!targetItems) {
  targetItems = {};
}

Consider tightening the guard to only replace null/undefined, not empty strings:

Suggested change
if (typeof defaultLocaleSourceItem === 'string') {
const translatedValue =
typeof targetItems === 'string'
? targetItems
: defaultLocaleSourceItem;
sourceObjectValue[mutateSourceItemKey] = translatedValue;
continue;
}
// If the source item is a string, use the translated string directly
if (typeof defaultLocaleSourceItem === 'string') {
const translatedValue =
typeof targetItems === 'string'
? targetItems
: defaultLocaleSourceItem;
sourceObjectValue[mutateSourceItemKey] = translatedValue;
continue;
}

The real fix belongs at the !targetItems initialisation (line 331):

// Before (replaces "" with {}):
if (!targetItems) { targetItems = {}; }

// After (only replaces null/undefined):
if (targetItems == null) { targetItems = {}; }

This keeps "" as a string so the downstream typeof targetItems === 'string' check works correctly.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/cli/src/formats/json/mergeJson.ts
Line: 347-354

Comment:
**`targetItems` replaced before string check**

The `!targetItems` guard on line 331–333 replaces a falsy `targetItems` with `{}` before the new string path is reached. An empty-string translation (`""`) is falsy, so it gets silently overwritten with `{}`, causing `typeof targetItems === 'string'` to be `false` and the code to fall back to `defaultLocaleSourceItem`. This means an empty translated string in the source map (intentional or otherwise) is silently discarded and replaced with the default-locale value.

```ts
let targetItems = targetJson[sourceObjectPointer];
if (!targetItems) {
  targetItems = {};
}
```

Consider tightening the guard to only replace `null`/`undefined`, not empty strings:

```suggestion
        // If the source item is a string, use the translated string directly
        if (typeof defaultLocaleSourceItem === 'string') {
          const translatedValue =
            typeof targetItems === 'string'
              ? targetItems
              : defaultLocaleSourceItem;
          sourceObjectValue[mutateSourceItemKey] = translatedValue;
          continue;
        }
```

The real fix belongs at the `!targetItems` initialisation (line 331):
```ts
// Before (replaces "" with {}):
if (!targetItems) { targetItems = {}; }

// After (only replaces null/undefined):
if (targetItems == null) { targetItems = {}; }
```
This keeps `""` as a string so the downstream `typeof targetItems === 'string'` check works correctly.

How can I resolve this? If you propose a fix, please make it concise.


// If the target locale has a matching source item, use it to mutate the source item
// Otherwise, fallback to the default locale source item
const mutateSourceItem = structuredClone(defaultLocaleSourceItem);
const mutateSourceItemKey = matchingTargetItem.keyParentProperty;

// 3. Merge the target items with the source item (if there are transformations to perform)
const mergedItems = {
Expand Down
8 changes: 7 additions & 1 deletion packages/cli/src/formats/json/parseJson.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function parseJson(
// Construct lvl 2
const sourceObjectsToTranslate: Record<
string,
Record<string, Record<string, string>>
Record<string, Record<string, string>> | string
> = {};
Comment on lines 63 to 66
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 14, 2026

Choose a reason for hiding this comment

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

No tests for new string handling

There are no test cases in parseJson.test.ts, mergeJson.test.ts, or extractJson.test.ts covering the new string-value composite path (e.g. { translations: { en: "Hello", fr: "Bonjour" } }). Since this PR is specifically fixing a bug in this code path, a regression test covering the full parse → merge → extract round-trip for string-valued composite schemas would significantly improve confidence in the fix.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/cli/src/formats/json/parseJson.ts
Line: 63-66

Comment:
**No tests for new string handling**

There are no test cases in `parseJson.test.ts`, `mergeJson.test.ts`, or `extractJson.test.ts` covering the new string-value composite path (e.g. `{ translations: { en: "Hello", fr: "Bonjour" } }`). Since this PR is specifically fixing a bug in this code path, a regression test covering the full parse → merge → extract round-trip for string-valued composite schemas would significantly improve confidence in the fix.

How can I resolve this? If you propose a fix, please make it concise.

for (const [
sourceObjectPointer,
Expand Down Expand Up @@ -163,6 +163,12 @@ export function parseJson(
}
const { sourceItem } = matchingItem;

// If the source item is a string, use it directly
if (typeof sourceItem === 'string') {
sourceObjectsToTranslate[sourceObjectPointer] = sourceItem;
continue;
}

// Get the fields to translate from the includes
const flatten = filterStrings ? flattenJsonWithStringFilter : flattenJson;
const itemsToTranslate = flatten(sourceItem, sourceObjectOptions.include);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/generated/version.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
// This file is auto-generated. Do not edit manually.
export const PACKAGE_VERSION = '2.10.0';
export const PACKAGE_VERSION = '2.10.1';
Loading