feat: implement fetch, and write our first integration test #1386
Conversation
🦋 Changeset detectedLatest commit: 1ef5c5b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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/2615466229/npm-package-wrangler-1386You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1386/npm-package-wrangler-1386Or you can use npx https://prerelease-registry.developers.workers.dev/runs/2615466229/npm-package-wrangler-1386 dev path/to/script.js |
| @@ -0,0 +1 @@ | |||
| declare module "wrangler"; | |||
There was a problem hiding this comment.
Will remove once we're outputting types
rozenmd
force-pushed
the
test-unstable-dev
branch
2 times, most recently
from
ed20f16
to
fc9d99a
Compare
rozenmd
changed the title
packages/wrangler/src/dev/local.tsx
Outdated
| @@ -255,7 +255,7 @@ function useLocalWorker({ | |||
| }); | |||
| //TODO: instead of being lucky with spawn's timing, have miniflare-cli notify wrangler that it's ready in packages/wrangler/src/miniflare-cli/index.ts, after the mf.startScheduler promise resolves | |||
| if (onReady) { | |||
| await new Promise((resolve) => setTimeout(resolve, 200)); | |||
| await new Promise((resolve) => setTimeout(resolve, 5000)); | |||
There was a problem hiding this comment.
is this where the 200 ms delay came from? also now it's 5 seconds?
There was a problem hiding this comment.
Yes - since we haven't implemented message passing between the wrangler and miniflare processes, this is a hack which waits 5000ms for miniflare to be ready for requests. Will try lower values and see if they pass.
There was a problem hiding this comment.
Can be as low as 250ms, but it's flaky. 500ms seems to be a good spot for now.
| process.exit(0); | ||
| }, | ||
| fetch: async () => { | ||
| const port = args.port || config.dev.port || (await getLocalPort()); |
There was a problem hiding this comment.
getLocalPort might not return the same value as was used by the server
I think you'll have to somehow get the value out
There was a problem hiding this comment.
add a todo comment here, this will be critical to fix
rozenmd
force-pushed
the
test-unstable-dev
branch
from
90bbf33
to
0eecc80
Compare
| return new Promise<{ stop: () => void }>((resolve) => { | ||
| return new Promise<{ | ||
| stop: () => void; | ||
| fetch: (init?: RequestInit) => Promise<Response | undefined>; |
There was a problem hiding this comment.
feels weird to be using fetch(init?) instead of the actual standard of fetch(resource, init?), but we're not letting folks pick the resource after the worker has started (yet)...
rozenmd
force-pushed
the
test-unstable-dev
branch
from
0aa9f6b
to
bd3278d
Compare
| process.exit(0); | ||
| }, | ||
| fetch: async () => { | ||
| const port = args.port || config.dev.port || (await getLocalPort()); |
There was a problem hiding this comment.
add a todo comment here, this will be critical to fix
| @@ -255,7 +255,7 @@ function useLocalWorker({ | |||
| }); | |||
| //TODO: instead of being lucky with spawn's timing, have miniflare-cli notify wrangler that it's ready in packages/wrangler/src/miniflare-cli/index.ts, after the mf.startScheduler promise resolves | |||
| if (onReady) { | |||
| await new Promise((resolve) => setTimeout(resolve, 200)); | |||
| await new Promise((resolve) => setTimeout(resolve, 500)); | |||
There was a problem hiding this comment.
this is going to need some ipc magic to fix
There was a problem hiding this comment.
yeah looking into this now on a separate branch
Disable whitespaces for this change
Closes #1383
Closes #1384
Closes #1385