feat: add --ip argument for `wrangler pages dev` & set default IP to `0.0.0.0` by kimyvgy · Pull Request #1605 · cloudflare/workers-sdk

kimyvgy · 2022-08-03T09:00:31Z

What does this PR do

Add a new argument --ip for wrangler pages dev, defaults to 0.0.0.0
Change default IP of wrangler dev to 0.0.0.0 instead of localhost

> wrangler pages dev --help
wrangler pages dev [directory] [-- command..]

Develop your full-stack Pages application locally

Positionals:
  directory  The directory of static assets to serve  [string]
  command    The proxy command to run  [string]

Flags:
  -h, --help     Show help  [boolean]
  -v, --version  Show version number  [boolean]

Options:
      --local                                  Run on my machine  [boolean] [default: true]
      --ip                                     The IP address to listen on  [string] [default: "0.0.0.0"]
      --port                                   The port to listen on (serve from)  [number] [default: 8788]
      --proxy                                  The port to proxy (where the static assets are served)  [number]
      --script-path                            The location of the single Worker script if not using functions  [string] [default: "_worker.js"]
  -b, --binding                                Bind variable/secret (KEY=VALUE)  [array]
  -k, --kv                                     KV namespace to bind  [array]
  -o, --do                                     Durable Object to bind (NAME=CLASS)  [array]
      --live-reload                            Auto reload HTML pages when change is detected  [boolean] [default: false]
      --local-protocol                         Protocol to listen to requests on, defaults to http.  [choices: "http", "https"]
      --experimental-enable-local-persistence  Enable persistence for this session (only for local mode)  [boolean] [default: false]

changeset-bot · 2022-08-03T09:00:34Z

🦋 Changeset detected

Latest commit: 604e542

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
wrangler	Patch

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

WalshyDev

Please add a changeset

packages/wrangler/src/pages/dev.tsx

github-actions · 2022-08-03T09:05:05Z

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/2791374616/npm-package-wrangler-1605

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1605/npm-package-wrangler-1605

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/2791374616/npm-package-wrangler-1605 dev path/to/script.js

codecov · 2022-08-03T09:06:43Z

Codecov Report

Merging #1605 (b543937) into main (7ae059b) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head b543937 differs from pull request most recent head 604e542. Consider uploading reports for the commit 604e542 to get more accurate results

@@            Coverage Diff             @@
##             main    #1605      +/-   ##
==========================================
+ Coverage   82.02%   82.04%   +0.01%     
==========================================
  Files          87       87              
  Lines        5815     5815              
  Branches     1491     1491              
==========================================
+ Hits         4770     4771       +1     
+ Misses       1045     1044       -1

Impacted Files	Coverage Δ
packages/wrangler/src/dev.tsx	`82.63% <ø> (ø)`
packages/wrangler/src/pages/dev.tsx	`23.77% <ø> (ø)`
...ackages/wrangler/src/__tests__/helpers/mock-bin.ts	`100.00% <0.00%> (+5.26%)`	⬆️

kimyvgy · 2022-08-03T09:19:53Z

It works in my local server. The IP address is set to 0.0.0.0 instead of localhost.

.changeset/witty-planets-refuse.md

packages/wrangler/src/pages/dev.tsx

kimyvgy · 2022-08-03T10:24:27Z

@WalshyDev I make second commit to set default IP address to 0.0.0.0 for pages dev. It seems CI does not check PR again.

WalshyDev · 2022-08-03T10:29:23Z

@WalshyDev I make second commit to set default IP address to 0.0.0.0 for pages dev. It seems CI does not check PR again.

just needed to be approved. I kicked it off now. I think once wrangler dev is also defaulting to 0.0.0.0 this is good to go

kimyvgy · 2022-08-03T11:50:36Z

@WalshyDev I have been rebased to single git commit. This PR is also defaulting IP to 0.0.0.0 for wrangler dev. Please check it when you have free time.

WalshyDev

This looks good to me 👍

GregBrimble · 2022-08-03T13:55:43Z

I'm not super familiar with it, but I'm guessing the current default localhost behavior comes from this line: https://github.com/cloudflare/wrangler2/blob/9732fafa066d3a18ba6096cfc814a2831f4a7d0e/packages/wrangler/src/config/validation.ts#L360. I think we should probably just clean that up if we're now just relying on yargs to generate the default.

Totally fine by me, btw. But I'd like just one more stamp from the core Wrangler team before merging.

…`0.0.0.0` - Add new argument `--ip` for `wrangler pages dev`, default `0.0.0.0` - Change default IP to `0.0.0.0` instead of `localhost` refactor(pages): set default IP address from "localhost" to "0.0.0.0" Co-authored-by: Daniel Walsh <walshydev@gmail.com> refactor(wrangler/dev): set default IP to `0.0.0.0` instead of `localhost`

kimyvgy · 2022-08-03T18:03:21Z

I replace the current default localhost in this line to 0.0.0.0 instead of cleaning up. That will compatible with RawDevConfig type because of ip field is string | undefined.
https://github.com/cloudflare/wrangler2/blob/9732fafa066d3a18ba6096cfc814a2831f4a7d0e/packages/wrangler/src/config/validation.ts#L360

JacobMGEvans · 2022-08-04T20:49:23Z

What is the benefit in having the default 0.0.0.0 vs. 127.0.0.1 or the virtual interface localhost?

EDIT:

This broke my development environment where I develop in a VM guest and test the app in VM host.

I see...

kimyvgy · 2022-08-04T21:04:31Z

I think the default 0.0.0.0 is friendlier than 127.0.0.1 for VM/container. 🤔

JacobMGEvans

Adding this here as a reference to the change from 127.0.0.1 to localhost and the reasoning, in case it is necessary to revert this change to 0.0.0.0 later on.

https://github.com/cloudflare/wrangler2/blob/main/packages/wrangler/CHANGELOG.md#0021

awthwathje · 2022-08-09T21:28:49Z

My two cents on why 0.0.0.0 is better for a development server. You can never be sure that clients will connect from the same machine.

You can connect from a VM or outside of a VM, like in my case. You can connect from another device, for example while testing on a real phone.

It's better to allow all those connections and not assume that the whole development cycle is isolated on the local host.

kimyvgy requested a review from a team as a code owner August 3, 2022 09:00

WalshyDev reviewed Aug 3, 2022

View reviewed changes

packages/wrangler/src/pages/dev.tsx Outdated Show resolved Hide resolved

kimyvgy force-pushed the feat branch from d766b98 to 5cafc79 Compare August 3, 2022 09:15

kimyvgy requested review from petebacondarwin and threepointone as code owners August 3, 2022 09:15

WalshyDev reviewed Aug 3, 2022

View reviewed changes

.changeset/witty-planets-refuse.md Outdated Show resolved Hide resolved

kimyvgy force-pushed the feat branch from 5cafc79 to 40d7526 Compare August 3, 2022 09:56

WalshyDev reviewed Aug 3, 2022

View reviewed changes

packages/wrangler/src/pages/dev.tsx Outdated Show resolved Hide resolved

WalshyDev linked an issue Aug 3, 2022 that may be closed by this pull request

🐛 BUG: pages dev only listens on localhost now (2.0.23) #1554

Closed

kimyvgy changed the title ~~feat: add --ip argument for wrangler pages dev~~ feat: add --ip argument for wrangler pages dev & set default IP to 0.0.0.0 Aug 3, 2022

kimyvgy force-pushed the feat branch from b543937 to a9e08a2 Compare August 3, 2022 11:45

WalshyDev approved these changes Aug 3, 2022

View reviewed changes

kimyvgy force-pushed the feat branch from a9e08a2 to 604e542 Compare August 3, 2022 17:45

JacobMGEvans added the pages Relating to Pages label Aug 3, 2022

GregBrimble removed the pages Relating to Pages label Aug 4, 2022

JacobMGEvans approved these changes Aug 4, 2022

View reviewed changes

WalshyDev merged commit 9e632cd into cloudflare:main Aug 5, 2022

github-actions bot mentioned this pull request Aug 5, 2022

Version Packages #1594

Merged

Cherry mentioned this pull request Aug 10, 2022

🐛 BUG: default 0.0.0.0 wrangler dev address not routable in Windows #1652

Closed

mrbbot mentioned this pull request Oct 22, 2022

Security concern: listening on all accessible IP ports by default is a risk #2036

Closed

Lekensteyn mentioned this pull request Nov 13, 2023

🐛 BUG: Wrangler is still listening on the wildcard address even with --ip localhost #4430

Closed

Conversation

kimyvgy commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do

Uh oh!

changeset-bot bot commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

WalshyDev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kimyvgy commented Aug 3, 2022

Uh oh!

Uh oh!

Uh oh!

kimyvgy commented Aug 3, 2022

Uh oh!

WalshyDev commented Aug 3, 2022

Uh oh!

kimyvgy commented Aug 3, 2022

Uh oh!

WalshyDev left a comment

Choose a reason for hiding this comment

Uh oh!

GregBrimble commented Aug 3, 2022

Uh oh!

kimyvgy commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JacobMGEvans commented Aug 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kimyvgy commented Aug 4, 2022

Uh oh!

JacobMGEvans left a comment

Choose a reason for hiding this comment

Uh oh!

awthwathje commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kimyvgy commented Aug 3, 2022 •

edited

Loading

changeset-bot bot commented Aug 3, 2022 •

edited

Loading

github-actions bot commented Aug 3, 2022 •

edited

Loading

codecov bot commented Aug 3, 2022 •

edited

Loading

kimyvgy commented Aug 3, 2022 •

edited

Loading

JacobMGEvans commented Aug 4, 2022 •

edited

Loading

awthwathje commented Aug 9, 2022 •

edited

Loading