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

allow passing of RequestCredential for typescript client library #719

Conversation

PhakornKiong
Copy link
Contributor

@PhakornKiong PhakornKiong commented May 16, 2023

https://encoredev.slack.com/archives/C0356ERKBEH/p1684146428092659

Allow configuration of RequestInit so dev have ability to pass along more default configuration of the client.

User of the client library will be able to do and make sure cookies are passed for all subsequent request

const encoreClient = new Client(baseUrl(), requestInit: { credentials: 'include' });

@encore-cla
Copy link

encore-cla bot commented May 16, 2023

All committers have signed the CLA.

@eandre
Copy link
Member

eandre commented May 16, 2023

Thanks @PhakornKiong! You can update the golden files with GOLDEN_UPDATE=1 go test ./...

@PhakornKiong
Copy link
Contributor Author

PhakornKiong commented May 16, 2023

Thanks @PhakornKiong! You can update the golden files with GOLDEN_UPDATE=1 go test ./...

i just tried using codescape, and got this error (is it due to version of go), any idea?

root@codespaces-<id>:/workspaces/encore# GOLDEN_UPDATE=1 go test ./...
# encr.dev/pkg/option
pkg/option/option.go:43:9: val.Comparable undefined (type reflect.Value has no field or method Comparable)
pkg/option/option.go:44:14: val.Equal undefined (type reflect.Value has no field or method Equal)
note: module requires Go 1.20
# encr.dev/pkg/paths
pkg/paths/paths.go:89:18: undefined: filepath.IsLocal
pkg/paths/paths.go:236:24: undefined: strings.CutPrefix
note: module requires Go 1.20
FAIL    encr.dev/cli/daemon/dash [build failed]

Update:
I updated dockerfile in dev container to FROM golang:1.20 and it is fixed

@PhakornKiong PhakornKiong force-pushed the ts-clientgen/RequestCredentials branch from 54a052f to e45da9b Compare May 17, 2023 05:59
@@ -72,6 +72,9 @@ export interface ClientOptions {
*/
fetcher?: Fetcher

/** indicates whether the user agent should send or receive cookies from the other domain in the case of cross-origin requests. */
credentials?: RequestCredentials;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having thought about this some more, I think it would be better if we had a field request?: RequestInit instead, and we merge those options with the per-request data. That seems more flexible and would support other RequestInit fields at the same time, without needing to copy each of those fields to ClientOptions over time. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, let me try to update it

@PhakornKiong PhakornKiong requested a review from eandre May 25, 2023 08:37
@PhakornKiong PhakornKiong requested a review from eandre May 26, 2023 04:42
@eandre
Copy link
Member

eandre commented May 31, 2023

Thanks @PhakornKiong for doing this, I merged it via #737 as I had to make a few minor changes on top. I kept you as the author. Hope that's okay!

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