Skip to content

Support for credentials mode#89

Open
johakr wants to merge 1 commit into
mikeal:masterfrom
johakr:master
Open

Support for credentials mode#89
johakr wants to merge 1 commit into
mikeal:masterfrom
johakr:master

Conversation

@johakr

@johakr johakr commented Apr 2, 2020

Copy link
Copy Markdown

#88

@mikeal

This comment has been minimized.

@mikeal

mikeal commented Apr 13, 2020

Copy link
Copy Markdown
Owner

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

johakr commented Apr 14, 2020

Copy link
Copy Markdown
Author

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
Copy Markdown

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

biaocy commented Jun 16, 2022

Copy link
Copy Markdown

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.

@nitindudeja2107 nitindudeja2107 mentioned this pull request Dec 20, 2022
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