fix(Request): support custom "credentials" value

[kettanaito]

Motivation

I'm adopting this polyfill as a part of a large API change in Mock Service Worker to adhere better to the Fetch API specification (mswjs/interceptors#292, part of mswjs/msw#1404). While doing so, I've spotted that whenever I construct a Request instance, its credentials are always set to same-origin, ignoring any custom credentials value I may supply in request init.

It's absolutely possible and allowed to construct a Request instance with any credentials the consumer needs.

Context: I'm constructing Request instance for an intercepted XMLHttpRequest as a part of the @mswjs/interceptors library. When dealing with XHR, it controls the credentials behavior with its withCredentials?: boolean flag. I must different request credentials values based on that flag to respect the specification and keep the consumer's code consistent.

Changes

• Adds credentials to RequestState
• Updates Request.credentials getter to resolve the value from the internal state.
• Adds missing test suites for request credentials behaviors

[changeset-bot]

⚠️ No Changeset found

Latest commit: 41c3bc3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[kettanaito]

I will leave the changeset/version bump decision up to the maintainers. It should be a patch release, if you ask me.

[kettanaito]

Hey, @jacob-ebey 👋 Sorry for the direct ping. May I please ask you to look at this change? It's small but it would unblock me greatly with some of my open-source work. It'd also improve specification compliance of this polyfill, so everyone wins. I would really appreciate your review on this.

[kettanaito]

I also confirm that this is indeed the missing piece for my work. Here the state of tests once I build and link this fix in my package:

Test Suites: 42 passed, 42 total
Tests:       148 passed, 148 total

[jacob-ebey]

I believe I'm ok with this change. The issue I have is that credentials have cascading effects that are not currently handled in this implementation: https://fetch.spec.whatwg.org/

I'm going to have to review the spec more and make sure this doesn't have any huge pitfalls before I can commit to merging this.

[kettanaito]

@jacob-ebey, thank you for the initial review. I may propose another motivation behind this is that in the current form the Request from this package handles credentials differently from the window.Request to which it claims to be compatible. To make things clear: the issue is only that this polyfill disregards the RequestInit.credentials property entirely.

But I understand that respecting the credentials, while bringing this implementation closer to the spec, may open some new issues where the credentials-derived implementation may be missing. If you need any help with implementing any dependent things you discover just let me know. A big chunk of my work depends on this change so I'm open to making these improvements happen.

[kettanaito]

I've read through the spec and sharing some of my thoughts below.

What RequestCredentials affect

They affect how the server handles credentials in CORS scenarios. Its value affects runtime behavior and has no effect on static Request/Response declarations.

The spec also has another concept of "credentials" that refers to the username and password present in the request's URL. That is a different thing altogether and has no connection to RequestCredentials.

Default value

Contrary to what the specification says, the default value for request.credentials is always same-origin.

If input is a string, it defaults to "same-origin".
Source

That is not entirely true. Even if Request is constructed using URL or Request instances, which are not strings, the value for request.credentials will be "same-origin".

Compliance

Referring to the specification more closely, this change aims to fulfill paragraph 19 of the Request constructor logic:

19. If init["credentials"] exists, then set request’s credentials mode to it.
    Source

Conclusion

• RequestCredentials is one of the ways to affect how the server would process credentials (cookies) from an incoming request. It controls the runtime behavior of the server.
• RequestInit.credentials must be respected and set on the created Request instance, if present.
• There are no other behaviors that derive from RequestCredentials and could otherwise affect the existing implementation of this polyfill (given the polyfill doesn't have existing issues that do not relate to or come out from this change).

@jacob-ebey, let me know what you find in the specification. Perhaps I've missed something.

[kettanaito]

Hey, @jacob-ebey. I really hate to disturb you regarding this change, may I ask you if I can help you somehow to move this forward? You can find my read of the Fetch API spec in the comment above. As I've said, you have my assistance with this because I need this change to unblock a major improvement in one of the libraries I maintain.

Thanks!