Skip to content

Commit

Permalink
Remove "credentials" option from transports
Browse files Browse the repository at this point in the history
Signed-off-by: Timo Stamm <ts@timostamm.de>
  • Loading branch information
timostamm committed Sep 19, 2024
1 parent 33be09c commit a3f3a82
Show file tree
Hide file tree
Showing 12 changed files with 66 additions and 86 deletions.
8 changes: 4 additions & 4 deletions packages/connect-web-bench/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ usually do. We repeat this for an increasing number of RPCs.

| code generator | RPCs | bundle size | minified | compressed |
| -------------- | ---: | ----------: | --------: | ---------: |
| Connect-ES | 1 | 276,503 b | 176,399 b | 35,786 b |
| Connect-ES | 4 | 280,755 b | 179,501 b | 36,562 b |
| Connect-ES | 8 | 285,618 b | 183,932 b | 37,491 b |
| Connect-ES | 16 | 294,746 b | 191,556 b | 38,985 b |
| Connect-ES | 1 | 276,204 b | 176,252 b | 35,762 b |
| Connect-ES | 4 | 280,456 b | 179,354 b | 36,568 b |
| Connect-ES | 8 | 285,319 b | 183,785 b | 37,490 b |
| Connect-ES | 16 | 294,447 b | 191,409 b | 39,021 b |
| gRPC-Web | 1 | 876,563 b | 548,495 b | 52,300 b |
| gRPC-Web | 4 | 928,964 b | 580,477 b | 54,673 b |
| gRPC-Web | 8 | 1,004,833 b | 628,223 b | 57,118 b |
Expand Down
10 changes: 5 additions & 5 deletions packages/connect-web-bench/chart.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
38 changes: 17 additions & 21 deletions packages/connect-web/src/connect-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,6 @@ export interface ConnectTransportOptions {
*/
useBinaryFormat?: boolean;

/**
* Controls what the fetch client will do with credentials, such as
* Cookies. The default value is "same-origin". For reference, see
* https://fetch.spec.whatwg.org/#concept-request-credentials-mode
*/
credentials?: RequestCredentials;

/**
* Interceptors that should be applied to all calls running through
* this transport. See the Interceptor type for details.
Expand All @@ -112,6 +105,11 @@ export interface ConnectTransportOptions {

/**
* Optional override of the fetch implementation used by the transport.
*
* The following fetch options are used by default:
* - credentials: "same-origin"
* - redirect: "error"
* - mode: "cors"
*/
fetch?: typeof globalThis.fetch;

Expand All @@ -129,6 +127,12 @@ export interface ConnectTransportOptions {
defaultTimeoutMs?: number;
}

const fetchOptions: RequestInit = {
credentials: "same-origin",
redirect: "error",
mode: "cors",
};

/**
* Create a Transport for the Connect protocol, which makes unary and
* server-streaming methods available to web browsers. It uses the fetch
Expand Down Expand Up @@ -168,13 +172,8 @@ export function createConnectTransport(
stream: false,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(options.baseUrl, method),
init: {
method: "POST",
credentials: options.credentials ?? "same-origin",
redirect: "error",
mode: "cors",
},
header: requestHeader(
method.methodKind,
useBinaryFormat,
Expand Down Expand Up @@ -202,7 +201,8 @@ export function createConnectTransport(
}
const fetch = options.fetch ?? globalThis.fetch;
const response = await fetch(req.url, {
...req.init,
...fetchOptions,
method: req.requestMethod,
headers: req.header,
signal: req.signal,
body,
Expand Down Expand Up @@ -336,13 +336,8 @@ export function createConnectTransport(
stream: true,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(options.baseUrl, method),
init: {
method: "POST",
credentials: options.credentials ?? "same-origin",
redirect: "error",
mode: "cors",
},
header: requestHeader(
method.methodKind,
useBinaryFormat,
Expand All @@ -356,7 +351,8 @@ export function createConnectTransport(
next: async (req) => {
const fetch = options.fetch ?? globalThis.fetch;
const fRes = await fetch(req.url, {
...req.init,
...fetchOptions,
method: req.requestMethod,
headers: req.header,
signal: req.signal,
body: await createRequestBody(req.message),
Expand Down
38 changes: 17 additions & 21 deletions packages/connect-web/src/grpc-web-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,6 @@ export interface GrpcWebTransportOptions {
*/
interceptors?: Interceptor[];

/**
* Controls what the fetch client will do with credentials, such as
* Cookies. The default value is "same-origin". For reference, see
* https://fetch.spec.whatwg.org/#concept-request-credentials-mode
*/
credentials?: RequestCredentials;

/**
* Options for the JSON format.
* By default, unknown fields are ignored.
Expand All @@ -103,6 +96,11 @@ export interface GrpcWebTransportOptions {

/**
* Optional override of the fetch implementation used by the transport.
*
* The following fetch options are used by default:
* - credentials: "same-origin"
* - redirect: "error"
* - mode: "cors"
*/
fetch?: typeof globalThis.fetch;

Expand All @@ -114,6 +112,12 @@ export interface GrpcWebTransportOptions {
defaultTimeoutMs?: number;
}

const fetchOptions: RequestInit = {
credentials: "same-origin",
redirect: "error",
mode: "cors",
};

/**
* Create a Transport for the gRPC-web protocol. The protocol encodes
* trailers in the response body and makes unary and server-streaming
Expand Down Expand Up @@ -158,21 +162,17 @@ export function createGrpcWebTransport(
stream: false,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(options.baseUrl, method),
init: {
method: "POST",
credentials: options.credentials ?? "same-origin",
redirect: "error",
mode: "cors",
},
header: requestHeader(useBinaryFormat, timeoutMs, header, false),
contextValues: contextValues ?? createContextValues(),
message,
},
next: async (req: UnaryRequest<I, O>): Promise<UnaryResponse<I, O>> => {
const fetch = options.fetch ?? globalThis.fetch;
const response = await fetch(req.url, {
...req.init,
...fetchOptions,
method: req.requestMethod,
headers: req.header,
signal: req.signal,
body: encodeEnvelope(0, serialize(req.message)),
Expand Down Expand Up @@ -350,21 +350,17 @@ export function createGrpcWebTransport(
stream: true,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(options.baseUrl, method),
init: {
method: "POST",
credentials: options.credentials ?? "same-origin",
redirect: "error",
mode: "cors",
},
header: requestHeader(useBinaryFormat, timeoutMs, header, false),
contextValues: contextValues ?? createContextValues(),
message: input,
},
next: async (req) => {
const fetch = options.fetch ?? globalThis.fetch;
const fRes = await fetch(req.url, {
...req.init,
...fetchOptions,
method: req.requestMethod,
headers: req.header,
signal: req.signal,
body: await createRequestBody(req.message),
Expand Down
11 changes: 6 additions & 5 deletions packages/connect/src/interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ export interface RequestCommon<I extends DescMessage, O extends DescMessage> {
*/
readonly service: DescService;

/**
* HTTP method of the request. Server-side interceptors may use this value
* to identify Connect GET requests.
*/
readonly requestMethod: string;

/**
* Metadata related to the service method that is being called.
*/
Expand All @@ -160,11 +166,6 @@ export interface RequestCommon<I extends DescMessage, O extends DescMessage> {
*/
readonly url: string;

/**
* Optional parameters to the fetch API.
*/
readonly init: Exclude<RequestInit, "body" | "headers" | "signal">;

/**
* The AbortSignal for the current call.
*/
Expand Down
5 changes: 1 addition & 4 deletions packages/connect/src/protocol-connect/get-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,7 @@ export function transformConnectPostToGetRequest<
].forEach((h) => header.delete(h));
return {
...request,
init: {
...request.init,
method: "GET",
},
requestMethod: "GET",
url,
header,
};
Expand Down
10 changes: 3 additions & 7 deletions packages/connect/src/protocol-connect/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ export function createTransport(opt: CommonTransportOptions): Transport {
stream: false,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(opt.baseUrl, method),
init: {},
header: requestHeaderWithCompression(
method.methodKind,
opt.useBinaryFormat,
Expand Down Expand Up @@ -127,7 +127,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
}
const universalResponse = await opt.httpClient({
url: req.url,
method: req.init.method ?? "POST",
method: req.requestMethod,
header: req.header,
signal: req.signal,
body,
Expand Down Expand Up @@ -207,12 +207,8 @@ export function createTransport(opt: CommonTransportOptions): Transport {
stream: true,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(opt.baseUrl, method),
init: {
method: "POST",
redirect: "error",
mode: "cors",
},
header: requestHeaderWithCompression(
method.methodKind,
opt.useBinaryFormat,
Expand Down
12 changes: 4 additions & 8 deletions packages/connect/src/protocol-grpc-web/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ export function createTransport(opt: CommonTransportOptions): Transport {
stream: false,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(opt.baseUrl, method),
init: {},
header: requestHeaderWithCompression(
opt.useBinaryFormat,
timeoutMs,
Expand All @@ -99,7 +99,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
next: async (req: UnaryRequest<I, O>): Promise<UnaryResponse<I, O>> => {
const uRes = await opt.httpClient({
url: req.url,
method: "POST",
method: req.requestMethod,
header: req.header,
signal: req.signal,
body: pipe(
Expand Down Expand Up @@ -216,12 +216,8 @@ export function createTransport(opt: CommonTransportOptions): Transport {
stream: true,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(opt.baseUrl, method),
init: {
method: "POST",
redirect: "error",
mode: "cors",
},
header: requestHeaderWithCompression(
opt.useBinaryFormat,
timeoutMs,
Expand All @@ -236,7 +232,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
next: async (req: StreamRequest<I, O>) => {
const uRes = await opt.httpClient({
url: req.url,
method: "POST",
method: req.requestMethod,
header: req.header,
signal: req.signal,
body: pipe(
Expand Down
4 changes: 2 additions & 2 deletions packages/connect/src/protocol-grpc/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ export function createTransport(opt: CommonTransportOptions): Transport {
stream: false,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(opt.baseUrl, method),
init: {},
header: requestHeaderWithCompression(
opt.useBinaryFormat,
timeoutMs,
Expand Down Expand Up @@ -200,8 +200,8 @@ export function createTransport(opt: CommonTransportOptions): Transport {
stream: true,
service: method.parent,
method,
requestMethod: "POST",
url: createMethodUrl(opt.baseUrl, method),
init: {},
header: requestHeaderWithCompression(
opt.useBinaryFormat,
timeoutMs,
Expand Down
8 changes: 4 additions & 4 deletions packages/connect/src/protocol/invoke-implementation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe("transformInvokeImplementation()", () => {
[
(next) => async (req) => {
expect(req.stream).toEqual(false);
expect(req.init.method).toEqual("POST");
expect(req.requestMethod).toEqual("POST");
expect(req.service).toEqual(TestService);
expect(req.header.get("Key")).toEqual("Value");
expect(req.url).toEqual("https://example.com/foo");
Expand Down Expand Up @@ -139,7 +139,7 @@ describe("transformInvokeImplementation()", () => {
[
(next) => async (req) => {
expect(req.stream).toEqual(true);
expect(req.init.method).toEqual("POST");
expect(req.requestMethod).toEqual("POST");
expect(req.service).toEqual(TestService);
expect(req.header.get("Key")).toEqual("Value");
expect(req.url).toEqual("https://example.com/foo");
Expand Down Expand Up @@ -191,7 +191,7 @@ describe("transformInvokeImplementation()", () => {
[
(next) => async (req) => {
expect(req.stream).toEqual(true);
expect(req.init.method).toEqual("POST");
expect(req.requestMethod).toEqual("POST");
expect(req.service).toEqual(TestService);
expect(req.header.get("Key")).toEqual("Value");
expect(req.url).toEqual("https://example.com/foo");
Expand Down Expand Up @@ -251,7 +251,7 @@ describe("transformInvokeImplementation()", () => {
[
(next) => async (req) => {
expect(req.stream).toEqual(true);
expect(req.init.method).toEqual("POST");
expect(req.requestMethod).toEqual("POST");
expect(req.service).toEqual(TestService);
expect(req.header.get("Key")).toEqual("Value");
expect(req.url).toEqual("https://example.com/foo");
Expand Down
4 changes: 1 addition & 3 deletions packages/connect/src/protocol/invoke-implementation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,13 @@ function requestCommon<I extends DescMessage, O extends DescMessage>(
spec: MethodImplSpec<I, O>,
): RequestCommon<I, O> {
return {
requestMethod: context.requestMethod,
url: context.url,
signal: context.signal,
header: context.requestHeader,
method: spec.method,
service: spec.method.parent,
contextValues: context.values,
init: {
method: context.requestMethod,
},
};
}

Expand Down
Loading

0 comments on commit a3f3a82

Please sign in to comment.