Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

nornagon
Copy link
Contributor

@nornagon nornagon commented Sep 2, 2020

Description of Change

This adds a new option to net.request(), "credentials", which more closely matches the behavior of fetch() than does "useSessionCookies".

If "credentials" is specified it overrides "useSessionCookies", otherwise the default behavior remains unchanged.

Checklist

Release Notes

Notes: Added a new "credentials" option for net.request().

@nornagon nornagon requested a review from deepak1556 September 2, 2020 18:25
@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Sep 2, 2020
@nornagon
Copy link
Contributor Author

nornagon commented Sep 2, 2020

NB. this change depends on the current master roll: #24984

Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

Api looks better now 👍

Also useSessionCookies was added for #8891 where users where only concerned with whether the cookies are completely handled by the store or not. So this change is reasonable for that too.

@MarshallOfSound MarshallOfSound added the semver/major incompatible API changes label Sep 3, 2020
@nornagon
Copy link
Contributor Author

nornagon commented Sep 3, 2020

@MarshallOfSound this is not semver/major because it doesn't break old behavior. it'll be semver/major when we remove the old one.

@nornagon nornagon added semver/minor backwards-compatible functionality and removed semver/major incompatible API changes labels Sep 3, 2020
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Sep 3, 2020
@nornagon nornagon changed the title feat: [net] change "useSessionCookies" to "credentials" feat: [net] add "credentials" option to net.request Sep 16, 2020
@nornagon nornagon requested a review from deepak1556 September 16, 2020 19:19
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

LGTM

shell/browser/api/electron_api_url_loader.cc Outdated Show resolved Hide resolved
@jkleinsc
Copy link
Member

The API WG approved this at the Sept 21, 2020 meeting.

Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

This needs a rebase from master to fix the mac arm64 compile issues.

@nornagon nornagon requested a review from jkleinsc September 28, 2020 17:59
@nornagon
Copy link
Contributor Author

@jkleinsc ping

@jkleinsc jkleinsc merged commit 0e7d59d into master Sep 29, 2020
@release-clerk
Copy link

release-clerk bot commented Sep 29, 2020

Release Notes Persisted

Added a new "credentials" option for net.request().

zeeker999 pushed a commit to zeeker999/electron that referenced this pull request Mar 13, 2021
* feat: [net] add "credentials" option to net.request

* remove debugging log

* add tests
zeeker999 pushed a commit to zeeker999/electron that referenced this pull request Mar 13, 2021
* feat: [net] add "credentials" option to net.request

* remove debugging log

* add tests
zeeker999 pushed a commit to zeeker999/electron that referenced this pull request Apr 9, 2021
* feat: [net] add "credentials" option to net.request

* remove debugging log

* add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-review/approved ✅ semver/minor backwards-compatible functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.