Skip to content
This repository was archived by the owner on May 29, 2025. It is now read-only.

Conversation

@yoshdog
Copy link

@yoshdog yoshdog commented Sep 13, 2018

Cloudflare workers now includes Request Attributes. These attributes are accessible every request on a cloudflare worker via request.cf.

This PR allows us to set and get the request attribute from request.cf which we can use when writing our tests for a cloudflare worker script.

To set a cloudflare request attribute:

const request = new Request('https://example.com', {cf: {country: 'AUS'}})

To access the request attribute:

request.cf

Example of how it works during testing:
https://github.com/envato/sites-cloudflare-worker/pull/3/files#diff-feb5a52860b926eb3cbf73e6758f50baR7

Cloudflare workers now includes [Request Attributes](https://developers.cloudflare.com/workers/reference/request-attributes/). These attributes are accessible every request from the cf object. i.e: `request.cf`.

This PR allows us to set and get the request attribute from `request.cf`.

To set a cloudflare request attribute:

```js
const request = new Request('https://example.com', {cf: {country: 'AUS'}})
```

To access the request attribute:

```js
request.cf
```
this.mode = options.mode || this.mode || null
this.signal = options.signal || this.signal
this.referrer = null
this.cf = options.cf || {}

Choose a reason for hiding this comment

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

I think we need to be stricter about what is possible on the cf object otherwise the behaviour doesn't align with the Cloudflare implementation. My proposal here is that we drop any keys that are not defined in the controlling Cloudflare feature docs to ensure that if a key is set, it will remove it and cannot be asserted against. Without this, someone could any key to this object, test against it but not have it do anything within the worker environment.

Choose a reason for hiding this comment

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

+1

Copy link

@jacobbednarz jacobbednarz left a comment

Choose a reason for hiding this comment

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

This is a great start ⭐️ I think we need to be stricter on the implementation of what is available on this object though to prevent unnoticed regressions with the custom cf features.

@jacobbednarz
Copy link

Can we also update the name of this module to be something like envato-cf-fetch to be explicit about the intention?

Copy link

@petervandoros petervandoros left a comment

Choose a reason for hiding this comment

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

Looking good, but I wonder if we could use Object.defineProperty instead to add the Cloudflare property? Perhaps a simple jest fetch wrapper?

It would be good to add a section to the docs/readme about the Cloudflare specific features we're adding.

})

test('construct with Cloudflare Request Attribute', function() {
var request = new Request('https://fetch.spec.whatwg.org/', {cf: {country: 'AUS'}})

Choose a reason for hiding this comment

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

Nitpick: country code should be AU.

this.mode = options.mode || this.mode || null
this.signal = options.signal || this.signal
this.referrer = null
this.cf = options.cf || {}

Choose a reason for hiding this comment

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

+1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants