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

--open false as a cli arg does not prevent browser from opening #2552

Closed
3 tasks done
drobannx opened this issue Mar 16, 2021 · 6 comments
Closed
3 tasks done

--open false as a cli arg does not prevent browser from opening #2552

drobannx opened this issue Mar 16, 2021 · 6 comments

Comments

@drobannx
Copy link

⚠️ IMPORTANT ⚠️ Please do not ignore this template. If you do, your issue will be closed immediately.

Describe the bug

We are trying to use the cli argument --open false to prevent the browser from opening when starting up a server but it does not work. If we do add the config option in vite.config.ts it does work.

I tracked down the reason and this is due to a type mismatch when deciding whether to open the browser. Because the cli arg will always come through as a string 'false' will evaluate to truth in the block below.

if (options.open && !isRestart) {
  const path = typeof options.open === 'string' ? options.open : base;
  openBrowser(`${protocol}://${hostname}:${port}${path}`, true, server.config.logger);
}

My teammate and I did notice that depending on OS and browser default, the call in openBrowser will fail silently as well.

Reproduction

Any project that tries to use --open false as a cli arg.

System Info

  • vite version: 2.0.5
  • Operating System: OSx 10.15.7
  • Node version: v14.14.0
  • Package manager (npm/yarn/pnpm) and version: yarn v1.22.10

I am happy to open an MR if there is a specific direction to get this to work correctly. My first thought was just to check against 'false' if typeof options.open is a string and not even call openBrowser if that condition is met.

@github-actions
Copy link

Hello @drobannx. We totally like your proposal/feedback, welcome to send us a Pull Request for it. Please provide changelog/TypeScript/documentation/test cases if needed and make sure CI passed, we will review it soon. We appreciate your effort in advance and looking forward to your contribution!

@egoist
Copy link
Contributor

egoist commented Mar 20, 2021

Hi, use --no-open to set open to false.

Since Vite doesn't open the browser by default, I'm not sure why you use --open false in the first place.

@Shinigami92
Copy link
Member

Yeah as @egoist already said, I think we should not add the possibility to allow --open false but just use --no-open if really needed.
And also by default it does not open 🤷

@matias-capeletto I will give you the last word on this an feel free to close both, the issue and PR if you share the same opinion with us

@patak-dev
Copy link
Member

I agree that the extra complexity doesn't look good here. Now "true" and "false" are no longer valid paths after this change (no idea if that would ever be needed but I would prefer to avoid that branching)

@drobannx could you explain a bit more about your use case and why --no-open is not enough?

@drobannx
Copy link
Author

@matias-capeletto - --no-open is perfect for our use case, we weren't aware it was an option.

When we ran the vite --help command, we didn't see --no-open but this works perfectly.

What are your thoughts on an MR to update the docs and/or change the --help output to show --no-open as an option and remove boolean as a type to --open

@Shinigami92
Copy link
Member

@drobannx Would love to see (and review) a docu update
Could you create a PR for that? I think you don't need to create an extra issue for that, we will jump directly into the PR 🙂

I will close this issue for now 👍

@github-actions github-actions bot locked and limited conversation to collaborators Jul 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants