-
Notifications
You must be signed in to change notification settings - Fork 736
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
feat: implement fetch, and write our first integration test #1386
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
--- | ||
"wrangler": patch | ||
--- | ||
|
||
feat: implement fetch for wrangler's unstable_dev API, and write our first integration test. | ||
|
||
Prior to this PR, users of `unstable_dev` had to provide their own fetcher, and guess the address and port that the wrangler dev server was using. | ||
|
||
With this implementation, it's now possible to test wrangler, using just wrangler (and a test framework): | ||
|
||
```js | ||
describe("worker", async () => { | ||
const worker = await wrangler.unstable_dev("src/index.ts"); | ||
|
||
const resp = await worker.fetch(); | ||
|
||
expect(resp).not.toBe(undefined); | ||
if (resp) { | ||
const text = await resp.text(); | ||
expect(text).toMatchInlineSnapshot(`"Hello World!"`); | ||
} | ||
|
||
worker.stop(); | ||
} | ||
``` | ||
|
||
Closes #1383 | ||
Closes #1384 | ||
Closes #1385 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,34 @@ | ||
{ | ||
"name": "local-mode-tests", | ||
"version": "1.0.0", | ||
"private": true, | ||
"description": "", | ||
"main": "index.js", | ||
"scripts": { | ||
"check:type": "tsc", | ||
"test": "npx jest --forceExit" | ||
}, | ||
"devDependencies": { | ||
"@cloudflare/workers-types": "^3.2.0" | ||
}, | ||
"keywords": [], | ||
"author": "", | ||
"license": "ISC", | ||
"jest": { | ||
"restoreMocks": true, | ||
"testTimeout": 30000, | ||
"testRegex": ".*.(test|spec)\\.[jt]sx?$", | ||
"transform": { | ||
"^.+\\.c?(t|j)sx?$": [ | ||
"esbuild-jest", | ||
{ | ||
"sourcemap": true | ||
} | ||
] | ||
} | ||
} | ||
"name": "local-mode-tests", | ||
"version": "1.0.0", | ||
"private": true, | ||
"description": "", | ||
"main": "index.js", | ||
"scripts": { | ||
"check:type": "tsc", | ||
"test": "npx jest --forceExit" | ||
}, | ||
"devDependencies": { | ||
"@cloudflare/workers-types": "^3.2.0" | ||
}, | ||
"keywords": [], | ||
"author": "", | ||
"license": "ISC", | ||
"jest": { | ||
"restoreMocks": true, | ||
"testTimeout": 30000, | ||
"testRegex": ".*.(test|spec)\\.[jt]sx?$", | ||
"transformIgnorePatterns": [ | ||
"node_modules/(?!find-up|locate-path|p-locate|p-limit|p-timeout|p-queue|yocto-queue|path-exists|execa|strip-final-newline|npm-run-path|path-key|onetime|mimic-fn|human-signals|is-stream|get-port|supports-color|pretty-bytes)", | ||
"wrangler-dist/cli.js" | ||
], | ||
"transform": { | ||
"^.+\\.c?(t|j)sx?$": [ | ||
"esbuild-jest", | ||
{ | ||
"sourcemap": true | ||
} | ||
] | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export default { | ||
async fetch() { | ||
return new Response("Hello World!"); | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import wrangler from "wrangler"; | ||
|
||
describe("worker", () => { | ||
let worker: { | ||
fetch: (init?: RequestInit) => Promise<Response | undefined>; | ||
stop: () => void; | ||
}; | ||
|
||
beforeAll(async () => { | ||
//since the script is invoked from the directory above, need to specify index.js is in src/ | ||
worker = await wrangler.unstable_dev("src/basicModule.ts", { | ||
ip: "127.0.0.1", | ||
port: 1337, | ||
}); | ||
}); | ||
|
||
afterAll(async () => { | ||
worker.stop(); | ||
}); | ||
|
||
it("should invoke the worker and exit", async () => { | ||
const resp = await worker.fetch(); | ||
expect(resp).not.toBe(undefined); | ||
if (resp) { | ||
const text = await resp.text(); | ||
|
||
expect(text).toMatchInlineSnapshot(`"Hello World!"`); | ||
} | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
declare module "wrangler"; | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
import { startDev } from "../dev"; | ||
import { logger } from "../logger"; | ||
|
||
import type { RequestInit, Response } from "undici"; | ||
|
||
interface DevOptions { | ||
env?: string; | ||
ip?: string; | ||
|
@@ -14,13 +17,20 @@ interface DevOptions { | |
_: (string | number)[]; //yargs wants this | ||
$0: string; //yargs wants this | ||
} | ||
|
||
/** | ||
* unstable_dev starts a wrangler dev server, and returns a promise that resolves with utility functions to interact with it. | ||
* @param {string} script | ||
* @param {DevOptions} options | ||
*/ | ||
export async function unstable_dev(script: string, options: DevOptions) { | ||
logger.warn( | ||
`unstable_dev() is experimental\nunstable_dev()'s behaviour will likely change in future releases` | ||
); | ||
|
||
return new Promise<{ stop: () => void }>((resolve) => { | ||
return new Promise<{ | ||
stop: () => void; | ||
fetch: (init?: RequestInit) => Promise<Response | undefined>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. feels weird to be using |
||
}>((resolve) => { | ||
//lmao | ||
return new Promise<Awaited<ReturnType<typeof startDev>>>((ready) => { | ||
const devServer = startDev({ | ||
|
@@ -33,7 +43,7 @@ export async function unstable_dev(script: string, options: DevOptions) { | |
showInteractiveDevSession: false, | ||
}); | ||
}).then((devServer) => { | ||
resolve({ stop: devServer.stop }); | ||
resolve({ stop: devServer.stop, fetch: devServer.fetch }); | ||
}); | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import { watch } from "chokidar"; | |
import getPort from "get-port"; | ||
import { render } from "ink"; | ||
import React from "react"; | ||
import { fetch } from "undici"; | ||
import { findWranglerToml, printBindings, readConfig } from "./config"; | ||
import Dev from "./dev/dev"; | ||
import { getVarsForDev } from "./dev/dev-vars"; | ||
|
@@ -23,6 +24,7 @@ import { | |
|
||
import type { Config } from "./config"; | ||
import type { Route } from "./config/environment"; | ||
import type { RequestInit } from "undici"; | ||
import type { Argv, ArgumentsCamelCase } from "yargs"; | ||
|
||
interface DevArgs { | ||
|
@@ -505,9 +507,13 @@ export async function startDev(args: ArgumentsCamelCase<DevArgs>) { | |
watcher, | ||
stop: async () => { | ||
devReactElement.unmount(); | ||
await devReactElement.waitUntilExit(); | ||
await watcher?.close(); | ||
process.exit(0); | ||
}, | ||
fetch: async (init?: RequestInit) => { | ||
const port = args.port || config.dev.port || (await getLocalPort()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. getLocalPort might not return the same value as was used by the server There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ugh There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a todo comment here, this will be critical to fix |
||
const address = args.ip || config.dev.ip || "localhost"; | ||
|
||
return await fetch(`http://${address}:${port}/`, init); | ||
}, | ||
}; | ||
} finally { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is going to need some ipc magic to fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah looking into this now on a separate branch |
||
onReady(); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will remove once we're outputting types