tursodatabase / libsql-client-ts

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

Don't require isomorphic-fetch in environments with fetch #111

Open birtles opened 1 year ago

birtles commented 1 year ago

Requiring isomorphic-fetch swells the size of any packages / Lambdas using libsql by over 100%.

For example, here is the breakdown of a very simple Lambda using @libsql/client:

image

Looking at how these are related:

image

i.e. the primary dependencies introduced by isomorphic-fetch include:

Dependency Size
isomorphic-fetch (including node-fetch) 38.3Kb
whatwg-url (required by isomorphic-fetch via node-fetch) 45.1Kb
tr46 (required by whatwg-url) 294.3Kb
TOTAL 377.7Kb
Percent of node_modules 58.94%

I wonder what environments still need node-fetch anyway? Node 16 has been EOL'ed so Node can be assumed to have native fetch and I believe all versions of Deno and Bun have native fetch too.

Given the API already allows providing a fetch function, would it be possible to assume a native fetch and allow environments that don't have native fetch (or have a deficient/buggy version) to pass in isomorphic-fetch or somesuch as needed rather than have all users pay the cost of node-fetch and friends?

penberg commented 1 year ago

@birtles I would love to just assume native fetch() but I think we got bitten by it so many times that I am now afraid to do it... 🙈

janpio commented 1 year ago

How others do it: https://github.com/planetscale/database-js#custom-fetch-function

birtles commented 1 year ago

@birtles I would love to just assume native fetch() but I think we got bitten by it so many times that I am now afraid to do it... 🙈

Yeah, fair enough. I suppose doing a major version bump to align with PlanetScale (i.e. the breaking change is that you must supply a fetch implementation if there's no native one) is one possibility?

birtles commented 1 year ago

For what it's worth, supplying native fetch to the createClient constructor doesn't seem to work for me. I get:

{
    "errorType": "TypeError",
    "errorMessage": "Failed to parse URL from [object Request]",
    "stack": [
        "TypeError: Failed to parse URL from [object Request]",
        "    at Object.fetch (node:internal/deps/undici/undici:11576:11)"
    ]
}

My best guess is that because the Request object used inside the hrana client is from node-fetch and hence it fails the brand checks inside the native fetch. (At least that's how it would work in browsers, not sure about the node implementation.)

DaBigBlob commented 11 months ago

@birtles i was facing the same/similar issue. so i wrote a thin client for a large subset of hrana http api v3: https://github.com/DaBigBlob/libsql-stateless

birtles commented 11 months ago

@birtles i was facing the same/similar issue. so i wrote a thin client for a large subset of hrana http api v3: https://github.com/DaBigBlob/libsql-stateless

@DaBigBlob Looks very interesting! Unfortunately I don't think I could use GPL 2.0 code for this particular project. Furthermore, for the main Lambda I was trying to optimize I switched to Rust due to #110.

DaBigBlob commented 11 months ago

@DaBigBlob Looks very interesting! Unfortunately I don't think I could use GPL 2.0 code for this particular project. Furthermore, for the main Lambda I was trying to optimize I switched to Rust due to #110.

@birtles idk why i chose gpl2 lol. its mit now.