feat(cli): support --host and --open on preview command (closes #367)#835
feat(cli): support --host and --open on preview command (closes #367)#83550rayn wants to merge 1 commit intohistoire-dev:mainfrom
Conversation
|
|
✅ Deploy Preview for histoire-examples-svelte3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-examples-vue3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-controls ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Code Review
This pull request refactors the preview server implementation to utilize Vite's built-in preview functionality, replacing the previous custom server setup using connect and sirv. It also updates the CLI to support --open and --host options for the preview command. Feedback was provided to ensure that the new implementation respects existing Vite configuration settings for the preview server and to improve the robustness of the base URL resolution when specific network hosts are configured.
| const port = options.port ?? 6006 | ||
| const server = await preview({ | ||
| root: ctx.root, | ||
| base: ctx.resolvedViteConfig.base, | ||
| configFile: false, | ||
| build: { | ||
| outDir: ctx.config.outDir, | ||
| }, | ||
| preview: { | ||
| port, | ||
| host: options.host, | ||
| open: options.open, | ||
| strictPort: false, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
The current implementation hardcodes several preview options and ignores potential user configuration from the Vite config (such as https, proxy, or headers). It is better to respect the resolved Vite configuration while allowing CLI options to override it. Additionally, the default port should ideally consider the one defined in the configuration to ensure consistency with user settings.
| const port = options.port ?? 6006 | |
| const server = await preview({ | |
| root: ctx.root, | |
| base: ctx.resolvedViteConfig.base, | |
| configFile: false, | |
| build: { | |
| outDir: ctx.config.outDir, | |
| }, | |
| preview: { | |
| port, | |
| host: options.host, | |
| open: options.open, | |
| strictPort: false, | |
| }, | |
| }) | |
| const port = options.port ?? ctx.resolvedViteConfig.preview.port ?? 6006 | |
| const server = await preview({ | |
| root: ctx.root, | |
| base: ctx.resolvedViteConfig.base, | |
| configFile: false, | |
| build: { | |
| outDir: ctx.config.outDir, | |
| }, | |
| preview: { | |
| ...ctx.resolvedViteConfig.preview, | |
| port, | |
| host: options.host, | |
| open: options.open, | |
| strictPort: false, | |
| }, | |
| }) |
| }) | ||
|
|
||
| httpServer.on('error', onError) | ||
| const baseUrl = server.resolvedUrls?.local[0] ?? `http://localhost:${port}${ctx.resolvedViteConfig.base}` |
There was a problem hiding this comment.
The baseUrl calculation can be improved for better robustness. If host is set to a specific IP, local[0] might be empty. Checking network[0] as well ensures a valid URL is returned in more network configurations (e.g., when exposing to the network).
| const baseUrl = server.resolvedUrls?.local[0] ?? `http://localhost:${port}${ctx.resolvedViteConfig.base}` | |
| const baseUrl = server.resolvedUrls?.local[0] ?? server.resolvedUrls?.network[0] ?? `http://localhost:${port}${ctx.resolvedViteConfig.base}` |
histoire
@histoire/app
@histoire/controls
@histoire/plugin-nuxt
@histoire/plugin-percy
@histoire/plugin-screenshot
@histoire/plugin-svelte
@histoire/plugin-vue
@histoire/shared
@histoire/vendors
commit: |
Closes #367.
histoire previewnow accepts--hostand--open, matchinghistoire dev. Implementation delegates to Vite's built-inpreview()so host binding, network URL printing, browser opening, and port retry are handled by Vite directly.Verified end-to-end on
examples/vue3:Also tightened
dev's--host <host>to--host [host]so the value is explicitly optional in the help text (behavior unchanged — sade already handled the no-value case).Test plan
pnpm buildandpnpm testpasshistoire previewinvocations above against the vue3 examplehistoire dev --host,--host 127.0.0.1still workbuild.ts's internalstartPreview({}, ctx)(used for screenshot rendering) still works