Skip to content

Conversation

Cherry
Copy link
Contributor

@Cherry Cherry commented Jun 11, 2023

Fixes #2797
What this PR solves / how to test:

Previously, Pages Functions exposed context.data like this:

const data = {};

const context = {
	...,
	data
};

Which meant that users could set context.data.foo = false and it would work fine, since it's just a reference to the top-level data per request, but the second they tried to override context.data entirely, such as context.data = {foo: false}, the reference to the original data is lost, so this data wouldn't actually show up.

This PR fixes that by adding a getter/setter pair to data, so that folks can override this without any issues.

Author has included the following, where applicable:

Reviewer is to perform the following, as applicable:

  • Checked for inclusion of relevant tests
  • Checked for inclusion of a relevant changeset
  • Checked for creation of associated docs updates
  • Manually pulled down the changes and spot-tested

@changeset-bot
Copy link

changeset-bot bot commented Jun 11, 2023

🦋 Changeset detected

Latest commit: 42946ec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 11, 2023

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5237240505/npm-package-wrangler-3431

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/3431/npm-package-wrangler-3431

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5237240505/npm-package-wrangler-3431 dev path/to/script.js
Additional artifacts:
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5237240505/npm-package-cloudflare-pages-shared-3431

Note that these links will no longer work once the GitHub Actions artifact expires.

@codecov
Copy link

codecov bot commented Jun 11, 2023

Codecov Report

Merging #3431 (42946ec) into main (3e43e88) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3431      +/-   ##
==========================================
+ Coverage   75.39%   75.44%   +0.04%     
==========================================
  Files         182      182              
  Lines       11060    11060              
  Branches     2903     2903              
==========================================
+ Hits         8339     8344       +5     
+ Misses       2721     2716       -5     

see 3 files with indirect coverage changes

@WalshyDev WalshyDev requested a review from a team June 11, 2023 15:53
@Cherry Cherry force-pushed the fix/pages-context-data-set branch from acfb115 to 8f40c2f Compare June 11, 2023 15:55
Copy link
Member

@WalshyDev WalshyDev left a comment

Choose a reason for hiding this comment

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

Like this fix. Working well locally, but had a bit of an issue due to JS destructuring but sadly that's not easily solvable (Proxy could but Proxy... ugh).

Happy with this, it should unblock most people. Thanks for the tests too :)

@WalshyDev WalshyDev merged commit 68ba49a into cloudflare:main Jun 11, 2023
@github-actions github-actions bot mentioned this pull request Jun 11, 2023
@Cherry Cherry deleted the fix/pages-context-data-set branch June 11, 2023 20:52
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.

🐛 BUG: Pages middleware context.data not persisted if overwritten
2 participants