Skip to content
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

[Bug]: Storybook does not respect BROWSER env var if default browser is not chromium based. #21343

Closed
thtliife opened this issue Mar 2, 2023 · 5 comments · Fixed by #21344 or #21473
Closed
Assignees

Comments

@thtliife
Copy link
Contributor

thtliife commented Mar 2, 2023

Describe the bug

When using the environment variable BROWSER to configure the browser that storybook opens in, storybook will not respect this configuration if the users default browser is not set to a browser which the @aw-web-design/x-default-browser package does not resolve to either chrome, chromium or com.brave.browser.

To Reproduce

To reproduce

Set the system default browser to something not based on chromium

eg: safari

Clone the reproduction repo

mkdir repo-test
cd repo-test
git clone https://github.com/thtliife/storybook-specify-browser-repro

Install dependencies

yarn

Run storybook with the BROWSER environment variable set to something other than the default system browser:

BROWSER=firefox yarn storybook

or create a .env file in the root of the project with the following contents:

BROWSER=firefox

then run storybook

yarn storybook

System

Environment Info:

  System:
    OS: macOS 13.1
    CPU: (8) arm64 Apple M1
  Binaries:
    Node: 16.13.0 - ~/.tools/n/bin/node
    Yarn: 1.22.19 - ~/.tools/n/bin/yarn
    npm: 8.1.0 - ~/.tools/n/bin/npm
  Browsers:
    Chrome: 110.0.5481.177
    Firefox: 110.0.1
    Safari: 16.2

Additional context

The problem is caused by the open-in-browser utility not passing args to open when the default browser is not chromium.

If the default browser is a chromium browser, better-opn is used which does its own evaluation of environment variables, allowing the BROWSER variable to be evaluated.

@thtliife
Copy link
Contributor Author

thtliife commented Mar 5, 2023

@tmeasday, A force push from the pull bot removed my changes in the pr #21344
It ended up with no changed files.
Should I create another pr for this?

@tmeasday tmeasday reopened this Mar 5, 2023
@tmeasday
Copy link
Member

tmeasday commented Mar 5, 2023

I guess so-- something odd is happening here !

@thtliife
Copy link
Contributor Author

thtliife commented Mar 5, 2023

hahaha
no problems

@thtliife
Copy link
Contributor Author

thtliife commented Mar 6, 2023

there we go, fingers crossed this one merges ok 😉

@shilman
Copy link
Member

shilman commented May 23, 2023

Egads!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.1.0-alpha.21 containing PR #21473 that references this issue. Upgrade today to the @future NPM tag to try it out!

npx sb@next upgrade --tag future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants