Skip to content

Conversation

@marcos-hairpieces
Copy link
Contributor

  • 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

What?

This pull request adds a new --serverHost CLI option to the stencil start command that allows developers to specify a custom hostname for the development server. The feature also supports configuration via the STENCIL_SERVER_HOST environment variable. This enhancement maintains full backward compatibility by defaulting to localhost when no custom host is specified.

The implementation propagates the serverHost configuration through the entire application stack:

  • CLI argument parsing and validation
  • BrowserSync proxy and host configuration
  • Server configuration via environment variable
  • Renderer plugin options for proper URL generation

This change is particularly valuable for Docker-based development environments where binding to 0.0.0.0 or a specific hostname is required for proper container networking, while preserving the existing localhost behavior for traditional development setups.

Tickets / Documentation

  • issue regarding the addition of this feature
  • README.md#running-in-docker provides 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

- 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',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants