Fix(db): update db-client connection options parsing

[Adammatthiesen]

Changes

• What does this change?
• Be short and concise. Bullet points can help!
• Before/after screenshots can help as well.
• Don't forget a changeset! pnpm exec changeset

This PR ensures proper params are being parsed and sent to the libsql Client, due to URLSearchParams being a Record<string, string> this object requires parsing to ensure the values are being parsed to number or boolean.

See https://discord.com/channels/830184174198718474/1370190284855705680 for context.

Testing

Added a new unit test for ensuring the URL parsing is correct in various circumstances.

Docs

This PR corrects internal logic to reflect the information on the docs. No docs changes needed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 637d863

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ematipico]

Can we add some tests @Adammatthiesen ?

[Adammatthiesen]

Can we add some tests @Adammatthiesen ?

Uhhh, how would I implement a test for this? 😅

Not exactly sure how I would setup that test... What this is being used for is setting up the libsql client to use an embedded replica of a remote server which creates a local (file or in memory) copy of the remote db that gets synced back and forth to the remote during runtime.

[ematipico]

Can we add some tests @Adammatthiesen ?

Uhhh, how would I implement a test for this? 😅

Not exactly sure how I would setup that test... What this is being used for is setting up the libsql client to use an embedded replica of a remote server which creates a local (file or in memory) copy of the remote db that gets synced back and forth to the remote during runtime.

We could export the function and they have a simple unit test that covers the basics. What worries me is that you haven't written anything under the testing section, so it's hard to understand what we're fixing and the fix works.

[Adammatthiesen]

Can we add some tests @Adammatthiesen ?

Uhhh, how would I implement a test for this? 😅
Not exactly sure how I would setup that test... What this is being used for is setting up the libsql client to use an embedded replica of a remote server which creates a local (file or in memory) copy of the remote db that gets synced back and forth to the remote during runtime.

We could export the function and they have a simple unit test that covers the basics. What worries me is that you haven't written anything under the testing section, so it's hard to understand what we're fixing and the fix works.

Your talking about the parse options function correct? To be clear, the issue is the following, currently if you have the following ENV variable from the astro docs:

ASTRO_DB_REMOTE_URL=file://local-copy.db?encryptionKey=your-encryption-key&syncInterval=60&syncUrl=libsql%3A%2F%2Fyour.server.io

it would be parsed as all strings for the config object, which would be invalid. Since the libSQLConfig object type wants the syncInterval needs to be a number for example. but it is currently being parsed as a string.

So current functionality would parse it as:

{
  url: "file://local-copy.db",
  encryptionKey: "your-encryption-key",
  syncInterval: "60",
  syncUrl: "libsql://your.server.io",
}

When it SHOULD be getting parsed as:

{
  url: "file://local-copy.db",
  encryptionKey: "your-encryption-key",
  syncInterval: 60, // <-- This is supposed to be a number according to the libSQL Config type
  syncUrl: "libsql://your.server.io",
}

[ematipico]

As for the test, I suggest the following:

• Extract the function outside createRemoteLibSQLClient
• The function will accept config and options too
• export the function
• Then create a new file inside this folder https://github.com/withastro/astro/tree/main/packages/db/test/unit, we can call it db-client.test.js
• When import the function from the dist/ folder e.g.

   import { parseOpts } from "../../dist/runtime/db-client.js"

• Test with assertions. We can use assert.deepEqual, and we use the example you shared with us
• The folder has other tests and assertions, you can use them as examples

Let me know if you have more questions

[Adammatthiesen]

As for the test, I suggest the following:

• Extract the function outside createRemoteLibSQLClient
• The function will accept config and options too
• export the function
• Then create a new file inside this folder https://github.com/withastro/astro/tree/main/packages/db/test/unit, we can call it db-client.test.js
• When import the function from the dist/ folder e.g.

   import { parseOpts } from "../../dist/runtime/db-client.js"

• Test with assertions. We can use assert.deepEqual, and we use the example you shared with us
• The folder has other tests and assertions, you can use them as examples

Let me know if you have more questions

Will get to work on that here soon, sorry passed out after i sent that last message 😅

[ematipico]

Thank you 🙏