Upgrade miniflare to 2.0.0-rc.5
#186
Conversation
🦋 Changeset detectedLatest commit: 66e3096 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 |
There was a problem hiding this comment.
This looks good to me, I'll let @GregBrimble have a look before merging it in.
| import { getType } from "mime"; | ||
| import open from "open"; | ||
| import { watch } from "chokidar"; | ||
|
|
||
| // Defer importing miniflare until we really need it. This takes ~0.5s | ||
| // and also modifies some `stream/web` and `undici` prototypes, so we | ||
| // don't want to do this if pages commands aren't being called. |
There was a problem hiding this comment.
could you explain why does miniflare modify undici/stream prototypes?
There was a problem hiding this comment.
For adding the non-standard getAll and readAtLeast methods to Headers and ReadableStreamBYOBReader:
We could probably return our own custom instances of these classes from Request/Response/ReadableStream subclasses, but this would be a bit of a pain, especially in the ReadableStream case as we'd also have to override TransformStream to return our ReadableStream. I think modifying the prototype is a cleaner solution in this case, especially since these are non-standard methods that shouldn't 🤞 conflict with anything else.
|
From a quick skim, this looks good. I'll try it out later today and if all continues to work as expected, I'll merge. |
| import { Headers, Request, Response } from "undici"; | ||
| import type { MiniflareOptions } from "miniflare"; | ||
| import type { RequestInfo, RequestInit } from "undici"; | ||
| import { URL } from "url"; |
There was a problem hiding this comment.
Don't think this is required. URL should be available in Node's global scope since v10 (https://nodejs.org/api/globals.html#url).
There was a problem hiding this comment.
I think this was to make TypeScript happy, seemed to be picking up the incorrect URL types for Miniflare otherwise
| }); | ||
| } catch (e) { | ||
| miniflare.log.error(e); | ||
| process.exitCode = 1; |
There was a problem hiding this comment.
This exit doesn't quite do enough to kill the process, but it's not blocking and likely will change with my #160 anyway.
Hey! 👋 This PR upgrades
miniflareto version2.0.0-rc.5: https://github.com/cloudflare/miniflare/releases/tag/v2.0.0-rc.5.This version implements service bindings and matches some other core behaviour of the Workers runtime, like blocking asynchronous I/O outside request handlers, and limiting request recursion depth. It also doesn't proxy primitive classes like
Objectby default, fixing #91. The old behaviour can be enabled with--proxy-primitivefor Rust workers. See this comment for more details.Local service bindings are not yet implemented in this PR. We need to have a discussion about how best to do these.
Normally, you'd just need to set the service
nameto bind too, since Cloudflare already knows all of your services.However, Miniflare needs to know where the other service's files are on disk, so it first requires you to mount (https://v2.miniflare.dev/core/mount) any services you want to bind.
I've made a few other minor changes to
pages dev:env.ASSETS.fetch.await generateAssetsFetch(directory)on every request, I've lifted it out. Every call togenerateAssetsFetchcreates a persistent watcher for_headersand_redirectsfiles, so this makes sure we only have one of those.miniflare.startServer()can throw if Miniflare initialisation fails for any reason (e.g. syntax error in user code). These errors are now logged.Headers,ResponseandRequestfrom@miniflare/coreinstead ofundici. This fixesenv.ASSETS.fetch(request)throwsTypeError: Failed to parse URL from [object Object]#165.