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

Support for credentials mode #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

johakr
Copy link

@johakr johakr commented Apr 2, 2020

#88

@mikeal

This comment has been minimized.

@mikeal
Copy link
Owner

mikeal commented Apr 13, 2020

Actually, I take that back, there’s isn’t much to do in Node.js.

I’d like to avoid an API here that has to touch core.js. It shouldn’t even need to extend the arguments if we do it right. How about something more like

const get = bent()
get.credentials = ‘include’

@johakr
Copy link
Author

johakr commented Apr 14, 2020

I understand your intention to keep core.js clean by only putting code there which is relevant to node and browser. On the other side, your proposal makes the client api more verbose and kinda kills bent's beauty of param type inspection for the browser. Probably, there is some trade-off solution in between, but I currently can't come up with one.

@timesync
Copy link

Just out of interest, any movement on the PR? I'm interested in using these changes but just wondering if I should implement them manually in my own repo rather than wait on this.

@biaocy
Copy link

biaocy commented Jun 16, 2022

Actually, I take that back, there’s isn’t much to do in Node.js.

I’d like to avoid an API here that has to touch core.js. It shouldn’t even need to extend the arguments if we do it right. How about something more like

const get = bent()
get.credentials = ‘include’

This get.credentials = 'include' workaround will not work, when use with another lib/framework which wrap window.fetch in the first place, like SvelteKit did.

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.

4 participants