fix: sqlite results should JSON.stringify
#1569
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JSON.stringify
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1569 +/- ##
=======================================
Coverage 39.84% 39.84%
=======================================
Files 769 769
Lines 37010 37014 +4
Branches 5230 5232 +2
=======================================
+ Hits 14745 14748 +3
- Misses 20499 20500 +1
Partials 1766 1766
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
fix(cli): add reachability metadata for `kotlin.Unit.toString()` fix(graalvm): make sqlite results read-only proxy objects fix(graalvm): make sqlite layer return undefined instead of unit test(graalvm): add test for sqlite JSON serialization test(cli): add smoke test for sqlite JSON serialization Fixes: #1356 Signed-off-by: Sam Gammon <sam@elide.dev>
6249376 to
0c2dea3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes SQLite result objects to properly stringify with JSON.stringify by making them ReadOnlyProxyObjects instead of regular proxy objects. The changes ensure SQLite query results can be serialized to JSON, which addresses issue #1356.
Key changes:
- Modified SQLite interfaces to extend
ReadOnlyProxyObjectfor proper JSON serialization - Updated method signatures to return
JSDynamicObjectinstead ofUnitfor consistency - Added comprehensive test coverage for JSON encoding/decoding of SQLite results
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tools/scripts/sqlite-json.mts |
New test script demonstrating SQLite JSON serialization |
packages/graalvm/src/test/kotlin/elide/runtime/gvm/internals/sqlite/SQLiteModuleTest.kt |
Added test for JSON encoding of SQLite results, removed redundant null checks |
packages/graalvm/src/main/kotlin/elide/runtime/intrinsics/sqlite/SQLiteObject.kt |
Made SQLiteObject extend ReadOnlyProxyObject for JSON compatibility |
packages/graalvm/src/main/kotlin/elide/runtime/intrinsics/sqlite/SQLiteDatabase.kt |
Updated to use ReadOnlyProxyObject and return JSDynamicObject from exec methods |
packages/graalvm/src/main/kotlin/elide/runtime/gvm/internals/sqlite/SqliteModule.kt |
Implemented proxy object methods for SQLiteObject and updated exec return types |
packages/graalvm/api/graalvm.api |
API changes reflecting interface updates |
packages/cli/src/test/kotlin/elide/tool/cli/ElideSmokeTests.kt |
Added new test script to smoke tests |
packages/cli/src/main/resources/META-INF/native-image/dev/elide/elide-cli/reachability-metadata.json |
Added SQLite and reflection metadata for native compilation |
packages/graalvm/src/main/kotlin/elide/runtime/gvm/internals/sqlite/SqliteModule.kt
Show resolved
Hide resolved
packages/graalvm/src/main/kotlin/elide/runtime/gvm/internals/sqlite/SqliteModule.kt
Show resolved
Hide resolved
JSON.stringifyJSON.stringify
darvld
approved these changes
Jul 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Makes
SQLiteObjectinstancesReadOnlyProxyObjects, which lets them stringify properly duringJSON.stringify.Changelog