Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: add named params support#92

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
ibilux wants to merge6 commits intoDallasHoff:main
base:main
Choose a base branch
Loading
fromibilux:main
Draft

Conversation

@ibilux
Copy link

Hello,

This is a work-in-progress for#91 aiming to add support for named parameters.
I’ve added tests, and they’re all passing so far. However, the current implementation is not perfect (far from it, actually), there’s definitely room for improvement.

Any feedback or suggestions for a cleaner or more efficient approach would be very welcome.

Thank you.

Signed-off-by: Bilux <i.bilux@gmail.com>
Signed-off-by: Bilux <i.bilux@gmail.com>
Signed-off-by: Bilux <i.bilux@gmail.com>
@DallasHoffDallasHoff marked this pull request as draftOctober 25, 2025 20:56
Copy link
Owner

@DallasHoffDallasHoffOct 25, 2025
edited
Loading

Choose a reason for hiding this comment

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

sqlTag andnormalizeSql should probably just be combined into one function. I'd look into replacingsqlTag in the places it is used with thenormalizeSql implementation. I'd keep thesqlTag naming though.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it would be better i think.

Copy link
Owner

@DallasHoffDallasHoff left a comment

Choose a reason for hiding this comment

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

This is looking really good so far.

@ibilux
Copy link
Author

This is looking really good so far.

What do you think about usingBindingSpec type instead ofunknown[] | Record<string, unknown> in theStatement params:

exporttypeStatement={sql:string;params:unknown[]|Record<string,unknown>;};

TheBindingSpec is thebind type indb.exec:

bind:statement.params,

Referenced from SQLite wasm:
https://github.com/sqlite/sqlite-wasm/blob/31e4b6e478a3f10a11b477feb08f1e54b53c791a/index.d.ts#L318

Signed-off-by: Bilux <i.bilux@gmail.com>
@ibiluxibiluxforce-pushed themain branch 3 times, most recently fromfc772d2 to17c2228CompareOctober 27, 2025 20:11
Signed-off-by: Bilux <i.bilux@gmail.com>
wip: testing the sqlite/wasm Bindable typing
@ibiluxibilux changed the titleWIP: add named params supportfeat: add named params supportOct 28, 2025
@ibilux
Copy link
Author

  • I have a updated the named parameters extraction method for all cases with more tests.
  • I have add tests for the numbered positional parameters?NNN.
  • The last commit is just for testing the sqlite/wasmBindable types (we can revert it if it's out of this PR scope).

@ibilux
Copy link
Author

  • I have a updated the named parameters extraction method for all cases with more tests.
  • I have add tests for the numbered positional parameters?NNN.
  • The last commit is just for testing the sqlite/wasmBindable types (we can revert it if it's out of this PR scope).

We can also export thenormalizeNamedParams as a helper function and give the user the option to chose if he wants to use the helper function of provide a valid named parameters object:

// Use the `normalizeNamedParams` helper functionawaitsql('INSERT INTO groceries (name) VALUES (:name)',normalizeNamedParams({name:'bread'}));// Or provide a valid binding object insteadawaitsql('INSERT INTO groceries (name) VALUES (:name)',{':name':'bread'});

This provide more flexibility, but may introduce some confusion.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@DallasHoffDallasHoffDallasHoff left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ibilux@DallasHoff

[8]ページ先頭

©2009-2025 Movatter.jp