-
Notifications
You must be signed in to change notification settings - Fork 778
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
Local Mode Console Support #2193
Conversation
🦋 Changeset detectedLatest commit: cda7e9d 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/3481535103/npm-package-wrangler-2193 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/2193/npm-package-wrangler-2193 Or you can use npx https://prerelease-registry.developers.workers.dev/runs/3481535103/npm-package-wrangler-2193 dev path/to/script.js Additional artifacts:npm install https://prerelease-registry.developers.workers.dev/runs/3481535103/npm-package-cloudflare-pages-shared-2193 |
Codecov Report
@@ Coverage Diff @@
## main #2193 +/- ##
==========================================
- Coverage 71.88% 71.79% -0.10%
==========================================
Files 145 145
Lines 9441 9452 +11
Branches 2459 2463 +4
==========================================
- Hits 6787 6786 -1
- Misses 2654 2666 +12
|
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.
Nice! :) Added some comments. I'm still a little confused how this works, as the Wrangler inspector proxy and the workerd
inspector are binding to the same port, but I've tested multiple Wrangler instances and seems fine. 😕
Added support for console logging when using miniflare3 resolves #2122
2057fd5
to
d9789b4
Compare
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.
Looks good, assuming CI passes ✅
ba0f368
to
cda7e9d
Compare
try { | ||
// fetch the inspector JSON response from the DevTools Inspector protocol | ||
const inspectorJSONArr = (await ( | ||
await fetch(`http://127.0.0.1:${inspectorPort}/json`) |
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.
FYI, this fetch()
fails on Node 16, which doesn't have fetch()
as a built-in yet. You'll have to import node-fetch
or change the package.json
to require Node 18.
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.
This was fixed in #2377, but it looks like this hasn't had a stable release yet. You should be able to npm install wrangler@beta
to use the main
branch.
Added support for console logging when using miniflare3
resolves #2122