feat(clients): Always order query params

.changeset/selfish-rings-mate.md

@@ -0,0 +1,6 @@
+---
+"@electric-sql/client": patch
+"@core/elixir-client": patch
+---
+
+Always use sorted query parameters in official clients to ensure Shape URLs are cached consistently.

packages/elixir-client/lib/electric/client/fetch/request.ex

@@ -80,7 +80,14 @@ defmodule Electric.Client.Fetch.Request do
     %{endpoint: endpoint} = request
 
     if Keyword.get(opts, :query, true) do
-      query = request |> params() |> URI.encode_query(:rfc3986)
+      # Convert map to _ordered_ list of query parameters
+      # to ensure consistent caching
+      query =
+        request
+        |> params()
+        |> Map.to_list()
+        |> List.keysort(0)
+        |> URI.encode_query(:rfc3986)
 
       URI.to_string(%{endpoint | query: query})
     else

packages/elixir-client/test/electric/client/fetch/request_test.exs

@@ -31,6 +31,9 @@ defmodule Electric.Client.Fetch.RequestTest do
 
       params = URI.decode_query(query)
 
+      # should have sorted parameters
+      assert query == "cursor=123948&handle=my-shape&live=true&offset=1234_1&table=my_table"
+
       assert %{
                "table" => "my_table",
                "cursor" => "123948",

packages/typescript-client/src/client.ts

@@ -358,6 +358,9 @@ export class ShapeStream<T extends Row<unknown> = Row>
           fetchUrl.searchParams.set(REPLICA_PARAM, this.#replica as string)
         }
 
+        // sort query params in-place for stable URLs and improved cache hits
+        fetchUrl.searchParams.sort()
+
         let response!: Response
         try {
           response = await this.#fetchClient(fetchUrl.toString(), {

packages/typescript-client/src/fetch.ts

@@ -312,6 +312,7 @@ function getNextChunkUrl(url: string, res: Response): string | void {
 
   nextUrl.searchParams.set(SHAPE_HANDLE_QUERY_PARAM, shapeHandle)
   nextUrl.searchParams.set(OFFSET_QUERY_PARAM, lastOffset)
+  nextUrl.searchParams.sort()
   return nextUrl.toString()
 }
 

packages/typescript-client/test/fetch.test.ts

@@ -218,7 +218,7 @@ describe(`createFetchWithChunkBuffer`, () => {
     expect(result).toBe(initialResponse)
 
     // Check if the next chunk was prefetched
-    const nextUrl = `${baseUrl}&handle=123&offset=456`
+    const nextUrl = sortUrlParams(`${baseUrl}&handle=123&offset=456`)
     expect(mockFetch).toHaveBeenCalledWith(nextUrl, expect.anything())
   })
 
@@ -250,23 +250,23 @@ describe(`createFetchWithChunkBuffer`, () => {
     expect(mockFetch).toHaveBeenCalledTimes(1 + maxPrefetchNum)
     expect(mockFetch).toHaveBeenNthCalledWith(
       2,
-      `${baseUrl}&handle=123&offset=0`,
+      sortUrlParams(`${baseUrl}&handle=123&offset=0`),
       expect.anything()
     )
     expect(mockFetch).toHaveBeenNthCalledWith(
       3,
-      `${baseUrl}&handle=123&offset=1`,
+      sortUrlParams(`${baseUrl}&handle=123&offset=1`),
       expect.anything()
     )
 
     // Second request consumes one of the prefetched responses and
     // next one fires up
-    await fetchWrapper(`${baseUrl}&handle=123&offset=0`)
+    await fetchWrapper(sortUrlParams(`${baseUrl}&handle=123&offset=0`))
     await sleep()
     expect(mockFetch).toHaveBeenCalledTimes(1 + maxPrefetchNum + 1)
     expect(mockFetch).toHaveBeenNthCalledWith(
       4,
-      `${baseUrl}&handle=123&offset=2`,
+      sortUrlParams(`${baseUrl}&handle=123&offset=2`),
       expect.anything()
     )
   })
@@ -297,7 +297,7 @@ describe(`createFetchWithChunkBuffer`, () => {
     expect(result).toBe(initialResponse)
 
     // fetch the next chunk as well
-    const nextUrl = `${baseUrl}&handle=123&offset=456`
+    const nextUrl = sortUrlParams(`${baseUrl}&handle=123&offset=456`)
     const nextResult = await fetchWrapper(nextUrl)
     expect(nextResult).toBe(nextResponse)
 
@@ -339,7 +339,8 @@ describe(`createFetchWithChunkBuffer`, () => {
     expect(result).toBe(initialResponse)
 
     // Prefetch should have been attempted but failed
-    const nextUrl = `${baseUrl}&handle=123&offset=456`
+    const nextUrl = sortUrlParams(`${baseUrl}&handle=123&offset=456`)
+
     expect(mockFetch).toHaveBeenCalledWith(nextUrl, expect.anything())
 
     // One for the main request, one for the prefetch
@@ -381,20 +382,20 @@ describe(`createFetchWithChunkBuffer`, () => {
     expect(mockFetch).toHaveBeenNthCalledWith(1, baseUrl)
     expect(mockFetch).toHaveBeenNthCalledWith(
       2,
-      `${baseUrl}&handle=123&offset=0`,
+      sortUrlParams(`${baseUrl}&handle=123&offset=0`),
       expect.anything()
     )
 
     // once interrupted it should have called the alt + the 2 prefetches
     expect(mockFetch).toHaveBeenNthCalledWith(3, altUrl)
     expect(mockFetch).toHaveBeenNthCalledWith(
       4,
-      `${altUrl}&handle=123&offset=2`,
+      sortUrlParams(`${altUrl}&handle=123&offset=2`),
       expect.anything()
     )
     expect(mockFetch).toHaveBeenNthCalledWith(
       5,
-      `${altUrl}&handle=123&offset=3`,
+      sortUrlParams(`${altUrl}&handle=123&offset=3`),
       expect.anything()
     )
   })
@@ -429,3 +430,9 @@ describe(`createFetchWithChunkBuffer`, () => {
     expect(mockFetch).toHaveBeenCalledTimes(2)
   })
 })
+
+function sortUrlParams(url: string): string {
+  const parsedUrl = new URL(url)
+  parsedUrl.searchParams.sort()
+  return parsedUrl.toString()
+}

packages/typescript-client/test/stream.test.ts

@@ -45,4 +45,35 @@ describe(`ShapeStream`, () => {
       },
     })
   })
+
+  it(`should sort query parameters for stable URLs`, async () => {
+    const eventTarget = new EventTarget()
+    const requestedUrls: Array<string> = []
+    const fetchWrapper = (
+      ...args: Parameters<typeof fetch>
+    ): Promise<Response> => {
+      requestedUrls.push(args[0].toString())
+      eventTarget.dispatchEvent(new Event(`fetch`))
+      return Promise.resolve(Response.error())
+    }
+
+    const aborter = new AbortController()
+    new ShapeStream({
+      url: shapeUrl,
+      table: `foo`,
+      where: `a=1`,
+      columns: [`id`],
+      handle: `potato`,
+      signal: aborter.signal,
+      fetchClient: fetchWrapper,
+    })
+
+    await new Promise((resolve) =>
+      eventTarget.addEventListener(`fetch`, resolve, { once: true })
+    )
+
+    expect(requestedUrls[0].split(`?`)[1]).toEqual(
+      `columns=id&handle=potato&offset=-1&table=foo&where=a%3D1`
+    )
+  })
 })