feat: add support for --local-protocol=https to wrangler dev
#580
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 0a36d4c 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/1976585367/npm-package-wrangler-580You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/580/npm-package-wrangler-580Or you can use npx https://prerelease-registry.developers.workers.dev/runs/1976585367/npm-package-wrangler-580 dev path/to/script.js |
petebacondarwin
force-pushed
the
dev-protocols
branch
2 times, most recently
from
e78d0b8 to
0c46580
Compare
4 tasks
petebacondarwin
changed the title
--local-protocol=https to wrangler dev
petebacondarwin
force-pushed
the
dev-protocols
branch
from
0c46580 to
97f5e0a
Compare
petebacondarwin
force-pushed
the
dev-protocols
branch
from
48946f3 to
4ead5b5
Compare
threepointone
approved these changes
Mar 13, 2022
There was a problem hiding this comment.
I went through all the commits, this looks totally great. I was surprised you did this so quickly! (I'm a noob at all the network bits, I learnt a fair bit from this PR). Chrome popping up the warning is unfortunate but unavoidable. Let's land this and give it a spin! Thank thank thank you!
Comment on lines
95
to
97
| fs.mkdirSync(CERT_CACHE_DIR, { recursive: true }); | ||
| fs.writeFileSync(keyPath, key, "utf8"); | ||
| fs.writeFileSync(certPath, cert, "utf8"); |
There was a problem hiding this comment.
This should have a guard around it so we can give a nicer error message if and when this fails because we don't have write perms to the directory
There was a problem hiding this comment.
well, it'll be dev and it would be weird if we don't have write perms, but still...
There was a problem hiding this comment.
Fair enough. I'll add that.
packages/wrangler/src/index.tsx
Outdated
| // The typings are not quite clever enough to handle element accesses, only property accesses, | ||
| // so we need to cast here. | ||
| (args["local-protocol"] as "http" | "https") || | ||
| config.dev.upstream_protocol |
There was a problem hiding this comment.
wait, shouldn't this be
Suggested change
| config.dev.upstream_protocol | |
| config.dev.local_protocol |
packages/wrangler/src/index.tsx
Outdated
| @@ -1263,6 +1269,7 @@ export async function main(argv: string[]): Promise<void> { | |||
| initialMode={args.local ? "local" : "remote"} | |||
| jsxFactory={envRootObj?.jsx_factory} | |||
| jsxFragment={envRootObj?.jsx_fragment} | |||
| localProtocol={config.dev.upstream_protocol} | |||
There was a problem hiding this comment.
Suggested change
| localProtocol={config.dev.upstream_protocol} | |
| localProtocol={config.dev.local_protocol} |
…a selection of choices
… "http" or "https"
This internal change just makes the final `Config["dev"]` type expect to have non-optional properties, making it cleaner to use in code. This is done, similarly to the `Environment` type by introducing `DevConfig` and partial `RawDevConfig` types, which represent the runtime config and the `wrangler.toml` types respectively. The `normalizeAndValidateDev()` helper now converts from the `RawDevConfig` to the `DevConfig`.
petebacondarwin
force-pushed
the
dev-protocols
branch
from
4ead5b5 to
0d15bf7
Compare
This change adds full support for the setting the protocol that the localhost proxy server listens to. Previously, it was only possible to use `HTTP`. But now you can set it to `HTTPS` as well. To support `HTTPS`, Wrangler needs an SSL certificate. Wrangler now generates a self-signed certificate, as needed, and caches it in the `~/.wrangler/local-cert` directory. These certificates expire after 30 days and are regenerated by Wrangler as needed. Note that if you use HTTPS then your browser will complain about the self-signed and you must tell it to accept the certificate before it will let you access the page.
petebacondarwin
force-pushed
the
dev-protocols
branch
from
0d15bf7 to
0a36d4c
Compare
|
I fixed the issues, and added some tests, which actually found further issues! |
Merged
mrbbot
added a commit
that referenced
this pull request
Oct 31, 2023
Miniflare extracts a relative module "name" from module paths. On
Windows, this process lead to names with `\`s in them. This meant
imports had to look like `import { ... } from "dir\\thing.js"` which
isn't right. This change normalises all import names to contain
forward slashes instead. Source mapping code is unaffected, as Node
will automatically normalise `/` to `\`s on Windows.
mrbbot
added a commit
that referenced
this pull request
Nov 1, 2023
Miniflare extracts a relative module "name" from module paths. On
Windows, this process lead to names with `\`s in them. This meant
imports had to look like `import { ... } from "dir\\thing.js"` which
isn't right. This change normalises all import names to contain
forward slashes instead. Source mapping code is unaffected, as Node
will automatically normalise `/` to `\`s on Windows.
mrbbot
added a commit
that referenced
this pull request
Nov 1, 2023
Miniflare extracts a relative module "name" from module paths. On
Windows, this process lead to names with `\`s in them. This meant
imports had to look like `import { ... } from "dir\\thing.js"` which
isn't right. This change normalises all import names to contain
forward slashes instead. Source mapping code is unaffected, as Node
will automatically normalise `/` to `\`s on Windows.
mrbbot
added a commit
that referenced
this pull request
Nov 1, 2023
Miniflare extracts a relative module "name" from module paths. On
Windows, this process lead to names with `\`s in them. This meant
imports had to look like `import { ... } from "dir\\thing.js"` which
isn't right. This change normalises all import names to contain
forward slashes instead. Source mapping code is unaffected, as Node
will automatically normalise `/` to `\`s on Windows.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds full support for the setting the protocol that the localhost proxy server listens to.
Previously, it was only possible to use
HTTP. But now you can set it toHTTPSas well.To support
HTTPS, Wrangler needs an SSL certificate.Wrangler now generates a self-signed certificate, as needed, and caches it in the
~/.wrangler/local-certdirectory.These certificates expire after 30 days and are regenerated by Wrangler as needed.
Note that if you use HTTPS then your browser will complain about the self-signed and you must tell it to accept the certificate before it will let you access the page.
Closes #582