Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2: Remove "credentials" option from transports #1242

Merged
merged 2 commits into from
Sep 20, 2024

Conversation

timostamm
Copy link
Member

@timostamm timostamm commented Sep 19, 2024

Transports from @connectrpc/connect-web have the "credentials" option, which is passed to the fetch call. The option passes through interceptors as well, in the "init" property of intercepted requests. The "init" property allows to change other fetch options in interceptors.

We later added the "fetch" option to transports, which allows to override the implementation used by the transport. This option provides a more simple and homogeneous way to set options for the fetch API. For example:

createConnectTransport({
  baseUrl: "/",
  fetch: (input, init) => fetch(input, {...init, credentials: "include"}),
});

We are removing the "credentials" option in v2. The "init" property of intercepted requests is also removed. As a replacement to determine whether an incoming request is a Connect GET request in server-side interceptors, the property requestMethod: string is added to intercepted requests. This property is symmetrical to HandlerContext.requestMethod.

Signed-off-by: Timo Stamm <ts@timostamm.de>
"credentials" already defaults to "same-origin", and "mode" already defaults to "cors", so we do not need to set these values.

Signed-off-by: Timo Stamm <ts@timostamm.de>
@timostamm timostamm merged commit d42db7a into v2 Sep 20, 2024
44 of 46 checks passed
@timostamm timostamm deleted the tstamm/Remove-credentials-option-from-transports branch September 20, 2024 08:45
@timostamm timostamm mentioned this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants