Skip to content
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.

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

Make sure that the correct entrypoint file path is applied when bundling Pages applications #4676

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

dario-piotrowicz
Copy link
Member

@dario-piotrowicz dario-piotrowicz commented Jan 1, 2024

Fixes #4652

Copy link

changeset-bot bot commented Jan 1, 2024

🦋 Changeset detected

Latest commit: 96634f3

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

Copy link
Contributor

github-actions bot commented Jan 1, 2024

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/7493808436/npm-package-wrangler-4676

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7493808436/npm-package-wrangler-4676 dev path/to/script.js
Additional artifacts:
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7493808436/npm-package-miniflare-4676
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7493808436/npm-package-cloudflare-pages-shared-4676
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7493808436/npm-package-create-cloudflare-4676 --no-auto-update

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


wrangler@3.22.4 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20231218.1
workerd 1.20231218.0 1.20231218.0
workerd --version 1.20231218.0 2023-12-18

|

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

Copy link

codecov bot commented Jan 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (df71f26) 75.60% compared to head (96634f3) 75.71%.
Report is 10 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4676      +/-   ##
==========================================
+ Coverage   75.60%   75.71%   +0.11%     
==========================================
  Files         243      243              
  Lines       13096    13210     +114     
  Branches     3376     3393      +17     
==========================================
+ Hits         9901    10002     +101     
- Misses       3195     3208      +13     
Files Coverage Δ
packages/wrangler/src/pages/dev.ts 16.43% <ø> (ø)

... and 62 files with indirect coverage changes

@dario-piotrowicz dario-piotrowicz marked this pull request as ready for review January 1, 2024 15:22
@dario-piotrowicz dario-piotrowicz requested a review from a team as a code owner January 1, 2024 15:22
@petebacondarwin petebacondarwin added the e2e Run e2e tests on a PR label Jan 2, 2024
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

I think that entry.file should already be resolved by the time we hit this code.
In most cases it is. From looking up the call stack I think the problem is in packages/wrangler/src/pages/dev.ts: if the working directory is not defined then the script-path (aka singleWorkerScriptPath) is used without resolving it correctly.

@dario-piotrowicz
Copy link
Member Author

Thanks a bunch @petebacondarwin 🙂

I've applied the change in the packages/wrangler/src/pages/dev.ts hopefully that's what you had in mind please let me know how that looks 🙂

@dario-piotrowicz dario-piotrowicz force-pushed the issue-4652 branch 3 times, most recently from e9ded02 to cc410e7 Compare January 7, 2024 14:34
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

I tested this locally by checking out the PR and running the following in the pages-dev-proxy-with-script fixture:

pnpm wrangler pages dev --proxy=9999 --script-path=custom/script/path/index.js

This fails with an error without the fix and works after the fix.

@dario-piotrowicz dario-piotrowicz merged commit 078cf84 into main Jan 11, 2024
19 checks passed
@dario-piotrowicz dario-piotrowicz deleted the issue-4652 branch January 11, 2024 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e Run e2e tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 BUG: Could not resolve "_worker.js" when running wrangler pages dev with --proxy flag
2 participants