feat: Add JS resp-set-header and request-set-field bindings#272
feat: Add JS resp-set-header and request-set-field bindings#272
Conversation
|
Since this is generated, I don't think we care about the |
flaki
left a comment
There was a problem hiding this comment.
One question regarding the respSetHeader API, looks good otherwise.
api/typescript/src/bindings/env.d.ts
Outdated
| cacheGet(key: string, ident: number): number; | ||
| requestGetField(fieldType: FieldType, key: string, ident: number): number; | ||
| requestSetField(fieldType: FieldType, key: string, value: Uint8Array, ident: number): number; | ||
| respSetHeader(key: string, value: Uint8Array, ident: number): void; |
There was a problem hiding this comment.
Hmm any reason value does not simply take a string here? I would think that's more appropriate for header values 🤔
Rust seems to take also set_header(key: &str, val: &str).
|
Hmm, I'm running the latest sat version (v0.1.4 with Reactr v0.15.1) and created a fork of the API to test this but for some reason there doesn't seem to be an {"log_message":"(I) [\"log-msg\",\"fetch-url\",\"graphql-query\",\"cache-set\",\"cache-get\",\"request-get-field\",\"request-set-field\",\"get-static-file\",\"db-exec\",\"get-ffi-result\",\"add-ffi-var\",\"return-result\",\"return-error\",\"canonical_abi_realloc\",\"memory\"]","timestamp":"2022-06-24T00:57:26.413949269+03:00","level":3,"app":{"sat_version":"v0.1.4"},"scope":{"request_id":"a8e52ae8-4152-4be0-905f-82f7907f727f","ident":664900642}} |
|
@flaki Sorry I wasn't more direct about it—suborbital/javy#16 is required to make this work. Running into some build issues over there but once that's resolved that'll be in. |
Ah, right, it didn't even occur to me that this could be in Javy but ofc that makes perfect sense. (also I somehow thought the backend bits of this are already in place because of Rust, but of course we are not using wit-bindgen for Rust so...) |
|
@flaki Updated to use a string. |
|
All of the |
|
Riiight, sorry, so this is exposing the "middleware" API, a la #213 ? |
|
Yeah, exactly. |
No description provided.