-
-
Notifications
You must be signed in to change notification settings - Fork 41
fix: Keep hybrid object data #255
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.
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
Changes from all commits
58ebc85
843db89
89e753b
8ce124a
078f30d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(find:*)", | ||
| "Bash(grep:*)" | ||
| ] | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,10 +11,39 @@ namespace margelo::nitro::rnnitrosqlite { | |
| class HybridNitroSQLiteQueryResult : public HybridNitroSQLiteQueryResultSpec { | ||
| public: | ||
| HybridNitroSQLiteQueryResult() : HybridObject(TAG) {} | ||
| HybridNitroSQLiteQueryResult(SQLiteExecuteQueryResult&& result) : HybridObject(TAG), _result(std::move(result)) {} | ||
| HybridNitroSQLiteQueryResult(SQLiteQueryResults results, std::optional<double> insertId, double rowsAffected, | ||
| std::optional<SQLiteQueryTableMetadata> metadata) | ||
| : HybridObject(TAG), _insertId(insertId), _rowsAffected(rowsAffected), _results(std::move(results)), _metadata(metadata) { | ||
| if (_results.empty()) { | ||
| // Empty rows: empty vector, length 0, item callback always returns null | ||
| auto emptyItem = [](double /* idx */) -> std::shared_ptr<Promise<std::optional<SQLiteQueryResultRow>>> { | ||
| return Promise<std::optional<SQLiteQueryResultRow>>::async([]() -> std::optional<SQLiteQueryResultRow> { return std::nullopt; }); | ||
| }; | ||
| _rows = NitroSQLiteQueryResultRows(SQLiteQueryResults{}, 0.0, std::move(emptyItem)); | ||
| return; | ||
| } | ||
|
|
||
| auto rows = _results; | ||
| auto itemFunction = [rows](double idx) -> std::shared_ptr<Promise<std::optional<SQLiteQueryResultRow>>> { | ||
| return Promise<std::optional<SQLiteQueryResultRow>>::async([rows, idx]() -> std::optional<SQLiteQueryResultRow> { | ||
| const auto index = static_cast<size_t>(idx); | ||
| if (index >= rows.size()) { | ||
| return std::nullopt; | ||
| } | ||
| return rows[index]; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Row index coercion changes item behaviorMedium Severity
|
||
| }); | ||
| }; | ||
|
|
||
| const auto length = static_cast<double>(rows.size()); | ||
| _rows = NitroSQLiteQueryResultRows(std::move(rows), length, std::move(itemFunction)); | ||
| } | ||
|
|
||
| private: | ||
| SQLiteExecuteQueryResult _result; | ||
| std::optional<double> _insertId; | ||
| double _rowsAffected; | ||
| SQLiteQueryResults _results; | ||
| std::optional<NitroSQLiteQueryResultRows> _rows; | ||
| std::optional<SQLiteQueryTableMetadata> _metadata; | ||
|
|
||
| public: | ||
| // Properties | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| import { HybridNitroSQLite } from '../nitro' | ||
| import type { NitroSQLiteQueryResult } from '../specs/NitroSQLiteQueryResult.nitro' | ||
| import type { QueryResult, QueryResultRow, SQLiteQueryParams } from '../types' | ||
| import NitroSQLiteError from '../NitroSQLiteError' | ||
|
|
||
|
|
@@ -10,7 +9,7 @@ export function execute<Row extends QueryResultRow = never>( | |
| ): QueryResult<Row> { | ||
| try { | ||
| const result = HybridNitroSQLite.execute(dbName, query, params) | ||
| return buildJsQueryResult<Row>(result) | ||
| return result as QueryResult<Row> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rows item contract changed unexpectedlyHigh Severity Returning Additional Locations (1) |
||
| } catch (error) { | ||
| throw NitroSQLiteError.fromError(error) | ||
| } | ||
|
|
@@ -23,25 +22,8 @@ export async function executeAsync<Row extends QueryResultRow = never>( | |
| ): Promise<QueryResult<Row>> { | ||
| try { | ||
| const result = await HybridNitroSQLite.executeAsync(dbName, query, params) | ||
| return buildJsQueryResult<Row>(result) | ||
| return result as QueryResult<Row> | ||
| } catch (error) { | ||
| throw NitroSQLiteError.fromError(error) | ||
| } | ||
| } | ||
|
|
||
| function buildJsQueryResult<Row extends QueryResultRow = never>( | ||
| result: NitroSQLiteQueryResult, | ||
| ): QueryResult<Row> { | ||
| const data = result.results as Row[] | ||
|
|
||
| return { | ||
| ...result, | ||
| insertId: result.insertId, | ||
| rowsAffected: result.rowsAffected, | ||
| rows: { | ||
| _array: data, | ||
| length: data.length, | ||
| item: (idx: number) => data[idx], | ||
| }, | ||
| } as QueryResult<Row> | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| import type { | ||
| NitroSQLiteQueryColumnMetadata, | ||
| NitroSQLiteQueryResult, | ||
| NitroSQLiteQueryResultRows, | ||
| } from './specs/NitroSQLiteQueryResult.nitro' | ||
|
|
@@ -42,17 +41,8 @@ export type QueryResultRow = Record<string, SQLiteValue> | |
|
|
||
| export type QueryResult<Row extends QueryResultRow = QueryResultRow> = | ||
| NitroSQLiteQueryResult & { | ||
| readonly rowsAffected: number | ||
| readonly insertId?: number | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generic QueryResult typing lost for resultsLow Severity
|
||
|
|
||
| /** Query results */ | ||
| readonly results: Row[] | ||
|
|
||
| /** Query results in a row format for TypeORM compatibility */ | ||
| readonly rows?: NitroSQLiteQueryResultRows<Row> | ||
|
|
||
| /** Table metadata */ | ||
| readonly metadata?: Record<string, NitroSQLiteQueryColumnMetadata> | ||
| rows?: NitroSQLiteQueryResultRows<Row> | ||
| } | ||
|
|
||
| export type ExecuteQuery = <Row extends QueryResultRow = QueryResultRow>( | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local Claude settings committed
Low Severity
The PR adds
.claude/settings.local.json, a machine-local configuration file unrelated to the SQLite feature change. Committing localpermissions.allowentries (Bash(find:*),Bash(grep:*)) can introduce environment-specific behavior and accidental policy drift in the repository.