-
-
Notifications
You must be signed in to change notification settings - Fork 246
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: enable wxt usage inside of devcontainers #1406
base: main
Are you sure you want to change the base?
The head ref may contain hidden characters: "feat/wxt-\u{1F91D}-devcontainers"
Conversation
❌ Deploy Preview for creative-fairy-df92c4 failed.
|
declare const __DEV_SERVER_PROTOCOL__: string; | ||
declare const __DEV_SERVER_HOSTNAME__: string; | ||
declare const __DEV_SERVER_PORT__: string; | ||
declare const __DEV_SERVER_ORIGIN__: string; |
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.
Instead of constructing a best-guess URL to use to contact the dev server, we can use ORIGIN
here
@@ -16,7 +16,8 @@ cli | |||
.option('-c, --config <file>', 'use specified config file') | |||
.option('-m, --mode <mode>', 'set env mode') | |||
.option('-b, --browser <browser>', 'specify a browser') | |||
.option('-p, --port <port>', 'specify a port for the dev server') | |||
.option('--host <host>', 'specify a host for the dev server to bind to') |
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.
Can't use -h
because that's already taken by --help
@@ -37,13 +44,9 @@ cli | |||
debug: flags.debug, | |||
filterEntrypoints: getArrayFromFlags(flags, 'filterEntrypoint'), | |||
dev: | |||
flags.port == null | |||
Object.keys(serverOptions).length === 0 | |||
? undefined |
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.
There's a test which requires this to be undefined
when neither host
nor port
are set
__DEV_SERVER_HOSTNAME__: JSON.stringify(server.hostname), | ||
__DEV_SERVER_PORT__: JSON.stringify(server.port), | ||
__DEV_SERVER_ORIGIN__: JSON.stringify( | ||
server.origin.replace(/^http(s):/, 'ws$1:'), |
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.
The regex replace converts http:
to ws:
and https:
to wss:
const originWithProtocolAndPort = [ | ||
origin.match(/^https?:\/\//) ? '' : 'http://', | ||
origin, | ||
origin.match(/:[0-9]+$/) ? '' : `:${port}`, | ||
].join(''); |
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.
As a convenience, the protocol and port are pre/appended when not given, but are left alone when specified by the user.
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.
Thanks for taking the time to write your PR! Looks good, I'll get this tested and merged soon.
For some history, I was always confused why Vite chose the name "host" instead of "hostname", because that was the part of the URL the string was used in. But that's not true, it's the host they're binding the server to, in networking terms, not in "parts of the URL" terms. And thus "hostname" was chosen incorrectly in WXT 🤦
Unfortunately, this PR is blocked by #1360 (comment), so it can't be merged right away - my goal is to get that fixed and merged within the next day. Essentially, I'm removing any code used to connect to the dev server manually, using import.meta.hot
instead. This change is to comply with the security updates in Vite 6.0.9. But there will likely be some conflicts with this PR.
I'm gonna give that fix priority over this one since it's security related. Not sure how it's going to effect this PR's behavior...
In the meantime, could you also add a FAQ question around how to run a WXT project inside docker? That would be really good to document!
https://github.com/wxt-dev/wxt/blob/main/docs/guide/resources/faq.md
Co-authored-by: Aaron <aaronklinker1@gmail.com>
Sounds great! I'll rebase and fix conflicts after you've had a chance to deal with #1360 (comment) Will also update the FAQ 😀 |
Overview
For users who want to run the dev server inside of a container, e.g. via orbstack or devcontainers, it's necessary to configure the dev server bind address separately to the dev server origin or websocket url.
Otherwise, the following error is thrown in the extension console and hot reloading is broken:
The bind address, hereafter referred to as
host
, is always going to be eitherlocalhost
/127.0.0.1
/::1
to lock it down to the local machine, or0.0.0.0
/::
to open it up to LAN traffic.Whereas the
origin
could belocalhost
,my-dev-computer.local
,some-dev-container.orb.local
, or all sorts of different configurations depending on the environment.The existing codebase conflates these two variables into the one
hostname
, which makes using wxt impossible in the above scenario.The full Discord discussion can be found here
Solution
The changes in this PR introduce the two new config parameters,
host
andorigin
, with an attempt to maintain backwards compatibility with existing user configurations (hostname
) as well as some sane but overridable defaults.hostname
has been replaced withhost
everywhere that the code is expecting a bind address.hostname
has been replaced withorigin
everywhere that the code is expecting a URL which the frontend can use to contact the dev server.hostname
config option will sethost
andorigin
just as it would have before these changes, but a deprecation notice will be served on the command line.host
is not configured, it will default tolocalhost
.origin
is not configured, it will default tohttp://localhost:<dev-server-port>
origin
is configured to a plain hostname, the protocol and port will be automatically added e.g.dev.local
->http://dev.local:<dev-server-port>
origin: "https://dev.local:9090"
for a proxy with TLS listening on port 9090Manual Testing
To test, run wxt inside of a docker container using e.g. docker desktop or orbstack. Make sure that the
.output
directory is bind-mounted to your host so that you can install the extension from the filesystem into Chrome.Once running, add the extension to Chrome and open the background console. Note that the console is unable to connect to the dev server.
Restart wxt inside of the container with the CLI argument
--host 0.0.0.0
and reload the extension in Chrome. Note that the dev server is now contactable, there are no CSP errors, and that hot reloading works as expected!Related Issues
This PR is a revision of the changes in #759 / #807