Skip to content

fix: preserve null for optional booleans and strip empty JSON objects in refineContentFields #3783

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

Open
surohak wants to merge 4 commits into
base: main
Choose a base branch
from
+20 −2
Open

fix: preserve null for optional booleans and strip empty JSON objects in refineContentFields #3783

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
72a12b0
fix: preserve null for optional booleans and strip empty JSON objects…
May 6, 2026
b279c22
fix: use Reflect.deleteProperty to satisfy no-dynamic-delete lint rule
May 6, 2026
4e2a322
fix: cast to object for Reflect.deleteProperty to satisfy TypeScript
May 6, 2026
c9abd1d
docs: add JSDoc to refineContentFields and helpers
May 6, 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
15 changes: 13 additions & 2 deletions src/runtime/internal/collection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,21 @@
const item = { ...doc } as T
for (const key in item) {
if (fields[key as string] === 'json' && item[key] && item[key] !== 'undefined') {
item[key] = JSON.parse(item[key] as string)
const parsed = JSON.parse(item[key] as string)
if (key !== 'meta' && parsed && typeof parsed === 'object' && !Array.isArray(parsed) && Object.keys(parsed).length === 0) {
delete item[key]

Check failure on line 11 in src/runtime/internal/collection.ts

View workflow job for this annotation

GitHub Actions / ubuntu 

Do not delete dynamically computed property keys
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
else {
item[key] = parsed
}
Comment on lines 13 to +20
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

'NULL' sentinel bypasses the JSON guard and throws SyntaxError

The guard at line 8 only excludes the string 'undefined'. The string 'NULL' is truthy and passes the check, so JSON.parse('NULL') is called — which throws a SyntaxError because uppercase NULL is not valid JSON. The 'NULL'→undefined normalization at lines 27–31 runs after this loop and cannot rescue the field.

Fix: extend the guard to exclude 'NULL', mirroring how 'undefined' is handled.

🐛 Proposed fix 
-    if (fields[key as string] === 'json' && item[key] && item[key] !== 'undefined') {
+    if (fields[key as string] === 'json' && item[key] && item[key] !== 'undefined' && item[key] !== 'NULL') {

As a secondary concern, JSON.parse at line 9 is still unchecked — any malformed JSON row will throw an unhandled SyntaxError. Consider wrapping in a try/catch or at least logging the error.

🤖 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 `@src/runtime/internal/collection.ts` around lines 8 - 15, The loop that parses
JSON fields (references: fields, item, key and the JSON.parse call) only guards
against the literal string 'undefined' so values equal to 'NULL' get passed to
JSON.parse and throw; update the guard to also exclude the string 'NULL' (i.e.,
treat item[key] === 'NULL' like 'undefined') and additionally wrap the
JSON.parse call in a try/catch so malformed JSON doesn't propagate a
SyntaxError—on parse failure either log the error and leave the original value
or set the field to undefined/omit it similarly to how 'undefined' is handled.

}
if (fields[key as string] === 'boolean' && item[key] !== 'undefined') {
item[key] = Boolean(item[key]) as never
if (item[key] == null) {
delete item[key]

Check failure on line 19 in src/runtime/internal/collection.ts

View workflow job for this annotation

GitHub Actions / ubuntu 

Do not delete dynamically computed property keys
}
else {
item[key] = Boolean(item[key]) as never
}
Comment on lines 22 to +28
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle 'NULL' sentinel before boolean coercion

At Line 22, Boolean(item[key]) will coerce 'NULL' to true. Since 'NULL' normalization happens later (Line 27+), this can leak wrong boolean values instead of omitting the optional field.

Treat 'NULL' as empty in the boolean branch before coercion.

Also applies to: 27-30

🧰 Tools
🪛 GitHub Check: ubuntu

[failure] 19-19:
Do not delete dynamically computed property keys

🤖 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 `@src/runtime/internal/collection.ts` around lines 17 - 23, The boolean
coercion branch in the collection normalization treats the sentinel string
'NULL' as truthy (using Boolean(item[key])), causing optional boolean fields to
be kept instead of omitted; update the logic in the boolean handling for
item[key] (the branch that checks fields[key] === 'boolean') to first treat the
exact string 'NULL' as empty (delete the key or treat as null) before any
Boolean(...) conversion, and apply the same sentinel check to the later
normalization block that handles null/undefined/'NULL' to ensure 'NULL' never
gets coerced to true; look for uses of fields, item and key in the boolean
branch and the subsequent normalization block and add the sentinel check there.

}
}

Expand Down
Loading