feat: add --serverHost CLI option for configurable dev server hostname#1337
feat: add --serverHost CLI option for configurable dev server hostname#1337marcos-hairpieces wants to merge 1 commit intobigcommerce:masterfrom
Conversation
- Add --serverHost option to stencil start command with localhost default - Support STENCIL_SERVER_HOST environment variable in server config - Pass serverHost through BrowserSync and renderer plugin configuration - Maintains backward compatibility with existing localhost behavior - Enables easier Docker configuration without disrupting defaults
| const watchIgnored = (watchOptions && watchOptions.ignored) || DEFAULT_WATCH_IGNORED; | ||
| this._browserSync.init({ | ||
| open: !!cliOptions.open, | ||
| host: cliOptions.serverHost || 'localhost', |
There was a problem hiding this comment.
Do we need here and below to default to localhost? I think commander should be doing it here
| themePath: this._themeConfigManager.themePath, | ||
| stencilCliVersion: PACKAGE_INFO.version, | ||
| storeSettingsLocale: this._storeSettingsLocale, | ||
| host: cliOptions.serverHost, |
There was a problem hiding this comment.
Bug: startLocalServer and _browserSync.init incorrectly use cliOptions.serverHost instead of cliOptions.host for server binding.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The startLocalServer function at lib/stencil-start.js:152 and _browserSync.init at lib/stencil-start.js:217 incorrectly attempt to read cliOptions.serverHost. The bin/stencil-start.js script maps the --serverHost CLI argument to options.host, not options.serverHost. Consequently, the host value passed to Server.create() and _browserSync.init() will be undefined, causing the server to default to 'localhost' instead of the user-specified value. This prevents binding to custom hostnames like 0.0.0.0.
💡 Suggested Fix
Modify lib/stencil-start.js at lines 152 and 217 to use cliOptions.host instead of cliOptions.serverHost when configuring the server and BrowserSync.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: lib/stencil-start.js#L152
Potential issue: The `startLocalServer` function at `lib/stencil-start.js:152` and
`_browserSync.init` at `lib/stencil-start.js:217` incorrectly attempt to read
`cliOptions.serverHost`. The `bin/stencil-start.js` script maps the `--serverHost` CLI
argument to `options.host`, not `options.serverHost`. Consequently, the `host` value
passed to `Server.create()` and `_browserSync.init()` will be `undefined`, causing the
server to default to `'localhost'` instead of the user-specified value. This prevents
binding to custom hostnames like `0.0.0.0`.
Did we get this right? 👍 / 👎 to inform future reviews.
What?
This pull request adds a new
--serverHostCLI option to thestencil startcommand that allows developers to specify a custom hostname for the development server. The feature also supports configuration via theSTENCIL_SERVER_HOSTenvironment variable. This enhancement maintains full backward compatibility by defaulting tolocalhostwhen no custom host is specified.The implementation propagates the serverHost configuration through the entire application stack:
This change is particularly valuable for Docker-based development environments where binding to
0.0.0.0or a specific hostname is required for proper container networking, while preserving the existing localhost behavior for traditional development setups.Tickets / Documentation
README.md#running-in-dockerprovides some steps on how to run stencil in docker, I've tried following those on both WSL and Linux with no success because of the lack of this feature, after patching it with this feature it worked(perhaps I was missing something and if that is the case I would really like to know)Screenshots (if appropriate)
N/A - This is a CLI configuration enhancement without visual changes.
cc @bigcommerce/storefront-team