tursodatabase / libsql-client-ts

TypeScript/JavaScript client API for libSQL
https://docs.turso.tech/sdk/ts/quickstart
MIT License
289 stars 37 forks source link

Use `URL` global in `parseUri` #162

Closed jokull closed 9 months ago

jokull commented 10 months ago

Reproduction

  1. Start a new next.js app router project (e.g. bun create next-app)
  2. Add app/test/route.ts
  3. Add this
import { createClient } from "@libsql/client/web";
import { NextResponse } from "next/server";

export const runtime = "edge";

export async function GET() {
  const statement = await createClient({
    url: "http://127.0.0.1:8080",
  }).execute("SELECT 1");
  return NextResponse.json(statement.toJSON());
}

Run bun run build.

jokull commented 9 months ago

Dug into some tests. I don't believe the global URL class will handle the following URL's found in tests without some special cases and RegEx detection:

  1. file://localhost - and its variants, because URL seems to have some special handling around not wanting to interpret hostnames
  2. file: with relative paths
  3. An empty path in any URL will get overwritten as the root / path

I'm no longer convinced my proposal is viable. It will need much more work to become backwards compatible and I don't have time for that right now.

giovannibenussi commented 9 months ago

Thanks a lot for your help with this. This is really useful for us because we still need to fix the issue with Next.js so your insights help us a lot!

I want to test out two different ideas. The first one is to look for an already built 3986 parser like elysiumphase/node-uri as a replacement and the second idea is to find a workaround for the regex.

penberg commented 9 months ago

Fixed by https://github.com/tursodatabase/libsql-client-ts/pull/172 so closing this PR.