Skip to content

Fix handling of "undefined"#370

Merged
kibertoad merged 3 commits intomainfrom
fix/null-vs-undefined
Dec 8, 2025
Merged

Fix handling of "undefined"#370
kibertoad merged 3 commits intomainfrom
fix/null-vs-undefined

Conversation

@kibertoad
Copy link
Copy Markdown
Owner

@kibertoad kibertoad commented Dec 7, 2025

Summary by CodeRabbit

  • Documentation

    • Clarified null vs undefined semantics and cache layer configuration guidance.
  • Bug Fixes

    • Cache now treats null as a resolved empty value (cached) and undefined as unresolved (not cached, continues querying).
  • Tests

    • Added tests validating that null updates cache and undefined does not; adjusted expectations accordingly.
  • Chores

    • Dependency bumps for runtime and dev tooling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Dec 7, 2025

Walkthrough

This PR separates null and undefined semantics: null is a resolved-but-empty value that is cached and stops further sources; undefined means unresolved, is not cached, and allows subsequent data sources to be queried. Tests, docs, and loader logic updated to reflect this.

Changes

Cohort / File(s) Summary
Documentation Updates
README.md
Clarified semantics: null = resolved-empty (cached, stops querying), undefined = unresolved (not cached, continue to next source). Added note that memory/async cache layers are optional and skipped if unconfigured.
Core Loader Logic
lib/Loader.ts, lib/GroupLoader.ts
Simplified per-source handling: only defined (non-undefined) results are cached and returned as-is (including null); undefined continues to next data source; after all sources, throw if throwIfUnresolved else return undefined. Removed final mapping of undefinednull.
Type Documentation
lib/types/DataSources.ts
Added JSDoc for DataSource and GroupDataSource describing return semantics and caching implications for null vs undefined (no signature changes).
Tests
test/Loader-main.spec.ts, test/GroupLoader-main.spec.ts
Added tests asserting: no cache update when all sources return undefined; cache set when a source returns explicit null; updated expectations where prior behavior returned null to now return undefined when unresolved.
Metadata / Tooling
package.json
Bumped dependency/devDependency versions: ioredis → ^5.8.2, vitest → ^3.2.4, typescript → ^5.9.3.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect loadFromLoaders paths in both lib/Loader.ts and lib/GroupLoader.ts to confirm only defined values are cached and returned.
  • Verify throw semantics after exhausting sources (respecting throwIfUnresolved).
  • Confirm tests in test/* reflect new expectations for undefined vs null.
  • Quick skim of README and JSDoc for accurate, consistent wording.

Poem

A rabbit peeks where loaders run,
Null hums "I'm found" beneath the sun,
Undefined skips on, nose to the breeze,
Caches stay tidy, queries find ease —
Hoppity-hop, the data's fun! 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix handling of "undefined"' directly addresses the core change: distinguishing between null (resolved-but-empty, cached) and undefined (unresolved, not cached) throughout the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/null-vs-undefined

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c596fd4 and 7356c40.

📒 Files selected for processing (3)
  • lib/GroupLoader.ts (2 hunks)
  • lib/Loader.ts (2 hunks)
  • package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • lib/GroupLoader.ts
  • lib/Loader.ts
🔇 Additional comments (1)
package.json (1)

53-53: No action needed for dependency version compatibility.

The dependency updates are conservative within their caret ranges (ioredis ^5.6.1→^5.8.2, vitest ^3.2.0→^3.2.4, typescript ^5.8.3→^5.9.3). Code inspection shows only standard, stable ioredis APIs in use (constructor, defineCommand, flushall, disconnect) with no deprecated patterns. These are routine maintenance updates with no impact on the null/undefined fix logic in the loader.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kibertoad kibertoad added the patch label Dec 8, 2025
@kibertoad kibertoad merged commit 0bcd356 into main Dec 8, 2025
7 checks passed
@kibertoad kibertoad deleted the fix/null-vs-undefined branch December 8, 2025 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants