feat: new rawBody type on SvelteKit req

[jthegedus]

Summary

Update to the latest SvelteKit request type.

Labelled as feat as this is breaking.

Fixes: #122

Details

The new type is Uint8Array, which we already get as a Buffer type (Node.js class which extends Uint8Array) from the import('firebase-functions').https.Request type, so this simplifies things a bit.

The assignment of headers is currently a type error:

Type 'IncomingHttpHeaders' is not assignable to type 'Headers'.
  Index signatures are incompatible.
    Type 'string | string[]' is not assignable to type 'string'.
      Type 'string[]' is not assignable to type 'string'.ts(2322)
internal.d.ts(18, 2): The expected type comes from property 'headers' which is declared here on type 'Incoming'

but looking at the Headers type in SvelteKit is seems this may not be an issue in future

It was an issue and worked around. deprecated code snippet here

// kit/types/helper.d.ts

// TODO we want to differentiate between request headers, which
// always follow this type, and response headers, in which
// 'set-cookie' is a `string[]` (or at least `string | string[]`)
// but this can't happen until TypeScript 4.3
export type Headers = Record<string, string>;

so I think it is okay to leave the IncomingHttpHeaders type for now as it hasn't been causing issues for people so far.

Thoughts @benmccann?

Tasks

• use Uint8Array for rawBody
• type render function
• remove string[] from headers
• unit test toSvelteKit* functions. Use requests generated from SvelteKit todos template?
  • test toSvelteKitRequest with GET
  • test toSvelteKitRequest with POST
• fix upstream type error. firebase-functions.https.request type is wrong as rawBody is optional as shown in upstream Node.js Functions Framework https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/e76b9f6c49bcf8c12d8206212cae84cfb931a077/src/invoker.ts#L44 - fix: rawBody is optional type in funcs framework firebase/firebase-functions#959
• improve integration tests to build from SvelteKit Todo app
• e2e test which makes a request to the Cloud Function & Hosting app using the Firebase Emulator in CI

[benmccann]

PR to fix that TODO here: sveltejs/kit#2248

I'm not sure why IncomingHttpHeaders accepts string[] values. Maybe it's only because they use the same structure for incoming and outgoing headers, but then I'm not sure why they have different types. I think it's only valid to have a string. If you have multiple values they're supposed to be comma-separated.

[jthegedus]

PR to fix that TODO here: sveltejs/kit#2248

Right, so Record<string, string> is the final type I should be targetting. The TODO was not to expand the type to Record<string, string|string[]> 👍

I am no expert of the Buffer/Uint8Array types, does rawBody: new Uint8Array(request.rawBody) seem okay to people. I couldn't find much on the web about creating a Uint8Array from a populated Buffer.

[jthegedus]

I'm not sure why IncomingHttpHeaders accepts string[] values. Maybe it's only because they use the same structure for incoming and outgoing headers, but then I'm not sure why they have different types. I think it's only valid to have a string. If you have multiple values they're supposed to be comma-separated.

It seems the only string[] on IncomingHttpHeaders is set-cookie. Is this an expected field on a request?

[benmccann]

The TODO was not to expand the type to Record<string, string|string[]>

Yeah. It was to change it to something like Record<string, string> & { 'Set-Cookie'?: string | string[] };. But that only sort of works sometimes even with TypeScript 4.3. I asked our liaison on the TypeScript team and he said it will probably never change

I am no expert of the Buffer/Uint8Array types, does rawBody: new Uint8Array(request.rawBody) seem okay to people. I couldn't find much on the web about creating a Uint8Array from a populated Buffer.

I'm not an expert on this either. All I could do is check the other adapters and Google search it

It seems the only string[] on IncomingHttpHeaders is set-cookie. Is this an expected field on a request?

No. I'm relatively sure you can only set that on responses

[jthegedus]

Great, thanks for the feedback and information @benmccann much appreciated.

[jthegedus]

Finally, the e2e test suite will:

• init a new SvelteKit todo template
• add the svelte-adapter-firebase & firebase config & functions/ dir with correct minimum config as in the docs
• hosts the built app on the Firebase Emulator in CI
• cURLs each of the pages /, /about, /todos and POSTs a new todo item to /todos

The below log shows the /todos SSR page rewriting to the Function, running its init code, hitting the Todo templates external API. It also shows the POST, listing it's requested resource as todos.json and the resultant payload showing successful creation of a new item.

[hosting] Rewriting /todos to http://localhost:5001/demo/us-central1/sveltekit for local Function sveltekit
i  functions: Beginning execution of "us-central1-sveltekit"
>  {"severity":"INFO","message":"Initialising SvelteKit SSR Handler"}
>  {"severity":"INFO","message":"SvelteKit SSR Handler initialised!"}
>  {"severity":"INFO","message":"Requested resource: /todos"}
⚠  External network resource requested!
   - URL: "api.svelte.dev/todos/bb7e4cad-bbd0-4326-ab27-f2e11796f478"
 - Be careful, this may be a production service.
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  2991    0  2991    0     0   4762      0 --:--:-- --:--:-- --:--:--  4755
100  2991    0  2991    0     0   4762      0 --:--:-- --:--:-- --:--:--  4755
====> Test POST to '/todos' API
i  hosting: 127.0.0.1 - - [23/Aug/2021:12:07:28 +0000] "GET /todos HTTP/1.1" 200 - "-" "curl/7.68.0"
i  functions: Finished "us-central1-sveltekit" in ~1s
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

[hosting] Rewriting /todos.json to http://localhost:5001/demo/us-central1/sveltekit for local Function sveltekit
i  functions: Beginning execution of "us-central1-sveltekit"
>  {"severity":"INFO","message":"Requested resource: /todos.json"}
⚠  External network resource requested!
   - URL: "api.svelte.dev/todos/0a52e7d5-25d4-4b12-b307-38756d00bbcb"
 - Be careful, this may be a production service.
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
i  hosting: 127.0.0.1 - - [23/Aug/2021:12:07:29 +0000] "POST /todos.json HTTP/1.1" 201 - "http://localhost:8685/todos" "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0"
i  functions: Finished "us-central1-sveltekit" in ~1s
100   277    0   100  100   177    398    705 --:--:-- --:--:-- --:--:--  1103
{"uid":"f21fb8bfa277444190d6636331cd178c2929","created_at":1629720449095,"text":"asdf","done":false}

🎉

If my new rawBody code is erroneous, at least we can update these tests to better capture why in future.