fix(pages): wrangler pages dev should prioritize _worker.js
#1950
Conversation
CarmenPopoviciu
requested review from
a team and
petebacondarwin
as code owners
🦋 Changeset detectedLatest commit: 3c0efbf The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/3181524686/npm-package-wrangler-1950You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1950/npm-package-wrangler-1950Or you can use npx https://prerelease-registry.developers.workers.dev/runs/3181524686/npm-package-wrangler-1950 dev path/to/script.jsAdditional artifacts:npm install https://prerelease-registry.developers.workers.dev/runs/3181524686/npm-package-cloudflare-pages-shared-1950 |
Codecov Report
@@ Coverage Diff @@
## main #1950 +/- ##
==========================================
- Coverage 72.72% 72.70% -0.02%
==========================================
Files 118 118
Lines 8230 8229 -1
Branches 2160 2160
==========================================
- Hits 5985 5983 -2
- Misses 2245 2246 +1
|
CarmenPopoviciu
force-pushed
the
pages-dev-prioritize-worker-over-fns
branch
2 times, most recently
from
637cd3d
to
23a294e
Compare
|
|
||
| if (url.pathname.startsWith("/date")) { | ||
| return new Response( | ||
| "Yesterday is history, tomorrow is a mystery, but today is a gift. That’s why it is called the present." |
| }; | ||
|
|
||
| await runBuild(); | ||
| watch([scriptPath], { |
There was a problem hiding this comment.
Does this watch only get cancelled if you kill the process?
There was a problem hiding this comment.
@petebacondarwin that is correct. In fact that is true for all our watchers. I am rather new to watch and esbuild so my reasoning might be wrong, but the way I see it is that we want to keep watching these files (_worker.js, /functions/* and _routes.json) for changes for as long as the process is running, so we can serve the right assets.
Is there a use case you had in mind that would require us to close the watcher? Or what was the underlying concern of your question?
There was a problem hiding this comment.
My only thought was whether esbuild kicks off a child process that might somehow get orphaned if we closed down the process in some other way than killing it? But I think that is probably not the case.
There was a problem hiding this comment.
according to evanw/esbuild#643 this 👉 https://github.com/evanw/esbuild/blob/master/cmd/esbuild/service.go#L124-L137 should prevent something like that from happening. So I think we should be OK
When using a _worker.js file, the entire /functions directory should be ignored – this includes its routing and middleware characteristics. Currently `wrangler pages dev` does the reverse, by prioritizing `/functions` over `_worker.js`. This commit fixes that.
CarmenPopoviciu
force-pushed
the
pages-dev-prioritize-worker-over-fns
branch
from
23a294e
to
3c0efbf
Compare
When using a _worker.js file, the entire
/functionsdirectory should be ignored – this includes its routing and middleware characteristics. Currentlywrangler pages devdoes the reverse, by prioritizing/functionsover_worker.js. This commit fixes that.