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

Allow server.open to be passed readonly string[] as per open spec #9572

Open
4 tasks done
mesqueeb opened this issue Aug 7, 2022 · 7 comments
Open
4 tasks done

Allow server.open to be passed readonly string[] as per open spec #9572

mesqueeb opened this issue Aug 7, 2022 · 7 comments
Labels
p2-to-be-discussed Enhancement under consideration (priority)

Comments

@mesqueeb
Copy link

mesqueeb commented Aug 7, 2022

Description

Currently, as per the vite types, server.open can only accept boolean | string

open?: boolean | string

Further more, the Vite docs say:

server.open
See the open package for more details.

When checking the Open docs, for best cross-platform support, they suggest to set the browser like so:

const open = require('open');

await open('https://google.com', {
	app: {
		name: open.apps.chrome
	}
});

Ref: https://github.com/sindresorhus/open#openapps

However, trying to use open.apps.chrome in Vite like so, gives a type error:

server: { open: open.apps.chrome }

image

Suggested solution

We need to be able to set Chrome but make sure it works for both Windows and MacOS.

My suggestion is to allow readonly string[] which is returned by open.apps.chrome so that we can use this option in the Vite config.

Alternative

No response

Additional context

No response

Validations

@mesqueeb mesqueeb changed the title Allow open to be passed readonly string[] as per open spec Allow server.open to be passed readonly string[] as per open spec Aug 7, 2022
@mesqueeb
Copy link
Author

mesqueeb commented Aug 7, 2022

related but different request:
#9557

@bluwy
Copy link
Member

bluwy commented Aug 16, 2022

In contrary to the docs, we actually have our own open implementation at https://github.com/vitejs/vite/blob/main/packages/vite/src/node/server/openBrowser.ts. So relaxing the types won't make it work. I'm not sure what's the reason of this mismatch though.

@bluwy
Copy link
Member

bluwy commented Aug 16, 2022

I've put this on the team board to discuss if we want to expand to using open instead of our own implementation. There might be a historical reason that I'm not aware of.

@mesqueeb
Copy link
Author

@bluwy thank for the information.

Another solution would be to delete this line from the docs:

See the open package for more details.

And the I'd love to know how to set it to specifically auto-open in Google Chrome, and in a cross-OS compatible way (Mac, Linux, Windows).

(Chrome is not my main browser but it's the browser I use for development.)

@sapphi-red
Copy link
Member

@bluwy

I'm not sure what's the reason of this mismatch though.

This is to reuse the existing tab. By using AppleScript, we can reuse it on macOS+Chromium base browsers. (It does not work with other OS/browers though...)
open will open a new tab everytime.

@mesqueeb
Copy link
Author

@sapphi-red why to open a PR towards open? Sindresorhus is a reasonable maintainer.

@bluwy
Copy link
Member

bluwy commented Aug 19, 2022

Thanks @sapphi-red for the find! I think it's nice if we can upstream the changes, we would probably have to have it as a normal dependency then, and re-export the app variable. But also, looks like Sindre prefers no to use an applescript: sindresorhus/open#256 (comment)

@patak-dev patak-dev added p2-to-be-discussed Enhancement under consideration (priority) and removed enhancement: pending triage labels Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-to-be-discussed Enhancement under consideration (priority)
Projects
Status: Has plan
Development

No branches or pull requests

4 participants