Skip to content

add support for assets bindings to getPlatformProxy (take 2) #7871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jun 25, 2025
Merged

add support for assets bindings to getPlatformProxy (take 2) #7871

merged 5 commits into from
Jun 25, 2025

Conversation

@dario-piotrowicz
Copy link
Member

@dario-piotrowicz dario-piotrowicz commented Jan 22, 2025 •
edited
Loading

Fixes #7812

this change makes sure that that getPlatformProxy, when the input configuration
file contains an assets field, correctly returns the appropriate asset binding proxy

This is reintroducing the change introduced in #7816 and reverted in #7866

This was reverted because some projects might call getPlatformProxy before the assets directory exists (for example if getPlatformProxy is used in their vite config, an initial vite build will cause the utility not to find the directory and throw an error), my solution is to try catch the exception and ignore the non-existing dir for this utility (this seems reasonable enough to me, I need to see if the team agrees)

@changeset-bot
Copy link

changeset-bot bot commented Jan 22, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: edc43d4

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

This PR includes changesets to release 2 packages
Name Type
wrangler Patch
@cloudflare/vitest-pool-workers 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

@dario-piotrowicz dario-piotrowicz changed the title add support for assets bindings to getPlatformProxy add support for assets bindings to getPlatformProxy (take 2) Jan 22, 2025
@dario-piotrowicz dario-piotrowicz added the e2e Run wrangler + vite-plugin e2e tests on a PR label Jan 22, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

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/13372236543/npm-package-wrangler-7871

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-wrangler-7871 dev path/to/script.js
Additional artifacts:

cloudflare-workers-bindings-extension:

wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-workers-bindings-extension-7871 -O ./cloudflare-workers-bindings-extension.0.0.0-vee9dc019c.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vee9dc019c.vsix

create-cloudflare:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-create-cloudflare-7871 --no-auto-update

@cloudflare/kv-asset-handler:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-kv-asset-handler-7871

miniflare:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-miniflare-7871

@cloudflare/pages-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-pages-shared-7871

@cloudflare/unenv-preset:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-unenv-preset-7871

@cloudflare/vite-plugin:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-vite-plugin-7871

@cloudflare/vitest-pool-workers:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-vitest-pool-workers-7871

@cloudflare/workers-editor-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-workers-editor-shared-7871

@cloudflare/workers-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-workers-shared-7871

@cloudflare/workflows-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13372236543/npm-package-cloudflare-workflows-shared-7871

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

[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20250204.1
workerd 1.20250204.0 1.20250204.0
workerd --version 1.20250204.0 2025-02-04

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@dario-piotrowicz dario-piotrowicz force-pushed the dario/7812/2/getplatformproxy_assets branch from b613d89 to 6b93b08 Compare January 22, 2025 22:33
vicb
vicb reviewed Jan 23, 2025
View reviewed changes
vicb
vicb reviewed Jan 23, 2025
View reviewed changes
vicb
vicb reviewed Jan 23, 2025
View reviewed changes
vicb
vicb reviewed Jan 23, 2025
View reviewed changes
vicb
vicb reviewed Jan 23, 2025
View reviewed changes
Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

Can you briefly explain in the description how this PR fixes the reason why the first iteration was reverted?

@dario-piotrowicz dario-piotrowicz force-pushed the dario/7812/2/getplatformproxy_assets branch 2 times, most recently from 6befada to 4daa1ab Compare January 23, 2025 18:57
@dario-piotrowicz
Copy link
Member Author

dario-piotrowicz commented Jan 23, 2025

Can you briefly explain in the description how this PR fixes the reason why the first iteration was reverted?

Done 👍

dario-piotrowicz
dario-piotrowicz commented Jan 23, 2025
View reviewed changes
vicb
vicb reviewed Jan 23, 2025
View reviewed changes
@penalosa
Copy link
Contributor

penalosa commented Feb 13, 2025

@dario-piotrowicz is this ready to pick up again?

@dario-piotrowicz
Copy link
Member Author

dario-piotrowicz commented Feb 13, 2025

@dario-piotrowicz is this ready to pick up again?

yes it is! it's been on the back of my mind for a bit but I've been busy with other stuff, I'll pick this back up tomorrow or in the next few days, sorry about it 🙇

@dario-piotrowicz dario-piotrowicz force-pushed the dario/7812/2/getplatformproxy_assets branch 3 times, most recently from 85f3177 to 1de49e9 Compare February 17, 2025 14:14
@dario-piotrowicz dario-piotrowicz marked this pull request as ready for review February 17, 2025 15:55
@dario-piotrowicz dario-piotrowicz requested a review from a team as a code owner February 17, 2025 15:55
@dario-piotrowicz
Copy link
Member Author

dario-piotrowicz commented Feb 17, 2025

@penalosa should be good to review now 🙂👍

vicb
vicb reviewed May 5, 2025
View reviewed changes
vicb
vicb approved these changes May 5, 2025
View reviewed changes
Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

LGTM with a couple nits

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk May 5, 2025
@dario-piotrowicz dario-piotrowicz force-pushed the dario/7812/2/getplatformproxy_assets branch from 1de49e9 to c5dbf1e Compare June 18, 2025 21:26
@github-actions
Copy link
Contributor

github-actions bot commented Jun 18, 2025

Failed to automatically backport this PR's changes to Wrangler v3. Please manually create a PR targeting the v3-maintenance branch with your changes. Thank you for helping us keep Wrangler v3 supported!

Depending on your changes, running git rebase --onto v3-maintenance main dario/7812/2/getplatformproxy_assets might be a good starting point.

Notes:

  • your PR branch should be named v3-backport-7871
  • add the skip-v3-pr label to the current PR to stop this workflow from failing

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jun 18, 2025 •
edited
Loading

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@7871

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@7871

miniflare

npm i https://pkg.pr.new/miniflare@7871

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@7871

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@7871

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@7871

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@7871

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@7871

wrangler

npm i https://pkg.pr.new/wrangler@7871

commit: edc43d4

@dario-piotrowicz dario-piotrowicz force-pushed the dario/7812/2/getplatformproxy_assets branch from c5dbf1e to 0deb955 Compare June 20, 2025 11:49
@dario-piotrowicz dario-piotrowicz mentioned this pull request Jun 20, 2025
Merged
@dario-piotrowicz dario-piotrowicz added the skip-v3-pr Skip validation of presence of a v3 backport PR label Jun 20, 2025
@dario-piotrowicz
Copy link
Member Author

dario-piotrowicz commented Jun 20, 2025 •
edited
Loading

Note

I've added the skip-v3-pr Skip validation of presence of a v3 backport PR label just so that the workflow doesn't fail there is a v3 backport PR associated to this

PS: I think that ideally the v3 backport check should be smart enough to understand when a manual backport PR has been created 🤔

@dario-piotrowicz dario-piotrowicz added this pull request to the merge queue Jun 25, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 25, 2025
@CarmenPopoviciu CarmenPopoviciu added this pull request to the merge queue Jun 25, 2025
Merged via the queue into main with commit f2a8d4a Jun 25, 2025
19 checks passed
@CarmenPopoviciu CarmenPopoviciu deleted the dario/7812/2/getplatformproxy_assets branch June 25, 2025 12:12
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Jun 25, 2025
CarmenPopoviciu added a commit that referenced this pull request Jun 25, 2025
@dario-piotrowicz @vicb @CarmenPopoviciu
V3 backport [#7871]: add support for assets bindings to `getPlatformP…
dacfc35 
…roxy` (#9694)

* add support for assets bindings to `getPlatformProxy`

* Apply suggestions from code review

Co-authored-by: Victor Berchet <[email protected]>

* fix formatting

* fix CI

* Update packages/wrangler/src/api/integrations/platform/index.ts

---------

Co-authored-by: Victor Berchet <[email protected]>
Co-authored-by: Carmen Popoviciu <[email protected]>
@workers-devprod workers-devprod mentioned this pull request Jun 25, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vicb vicb vicb approved these changes

Assignees

No one assigned

Labels

e2e Run wrangler + vite-plugin e2e tests on a PR skip-v3-pr Skip validation of presence of a v3 backport PR

Projects

workers-sdk
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

🚀 Feature Request: add ASSETS binding to getPlatformProxy

4 participants

@dario-piotrowicz @penalosa @vicb @CarmenPopoviciu